2012-03-02 13 views
0

ちょっと私の動的配列の内容をシャッフルしようとしているし、動作していない。 yallに助けやリンク/リソースがあれば私を助けることができますか?私はstd :: randomshuffleを使用しようとしていますが、私のテストは正しいデータの代わりに0を吐き出しています。動的配列のC++シャッフルコンテンツ?

Songs *ptr; 
ptr = new Songs[25]; 

ifstream fin; 
fin.open("input.txt"); 

while (fin.good())     //my input 
{ 
     getline(fin, song[num].title);  
     getline(fin, song[num].artist); 
     fin >> song[num].mem; 
     num++; 
     fin>>ws; 
} 
fin.close(); 

とHERESに私の機能イムは、ループ条件としてistream::good()またはistream::eof()は絶対に使用しないでくださいrandomshuffle

void shuffle (char choice, Songs song[], Songs *ptr, string title, string artist, int mem, int num) 
{ 
    if (choice == '4') 
    { 
     std::random_shuffle(ptr, ptr + num);    //shuffle 
    } 
    for (int i = 0; i<num; i++) //test 
    { 
     cout << ptr[i].title << ptr[i].artist << ptr[i].mem << endl; 
    } 
} 
+0

なぜ、すべてのパラメータがします'shuffle()'関数ですか?あなたは 'ptr'と' num'だけを使用しています。シャッフルするかどうかのロジックは、シャッフル機能の中で実際に行われるべきではありません。 – jrok

+0

よくそれらは私のptr配列の内容なので、私はそれらも含めなければならないと考えました – gamergirl22

+0

@ gamergirl22絶対に必要としないし、これを行うべきではありません。後でコードを効果的に設計する方法について考える必要があります。このシャッフル機能のポイントは、曲をシャッフルしてプリントアウトすることです。だから、私たちが必要とするのは、歌と数字だけです(それは「歌」と「ptr」ではない)も冗長です。 – stinky472

答えて

2

これは、最近のC++の問題へのアプローチです。あなたは手動で読みたいたびにオブジェクトを解析する必要はありませんので、ストリーム演算子を作成します。

#include <algorithm> 
#include <string> 
#include <iostream> 
#include <fstream> 
#include <vector> 
#include <iterator> 

struct song { 
     std::string title, artist; 
     int mem; 
}; 

std::ostream& operator<<(std::ostream& os, const song& s) { 
     return os << s.title << "\t" << s.artist << "\t" << s.mem; 
} 

std::istream& operator>>(std::istream& is, song& s) { 
     std::getline(is, s.title); 
     std::getline(is, s.artist); 
     return is >> s.mem; 
} 

int main() 
{ 
     std::ifstream file("input.txt"); 

     if(!file.is_open()) return 1; 

     std::vector<song> songs((std::istream_iterator<song>(file)), 
           std::istream_iterator<song>()); 
     std::random_shuffle(songs.begin(), songs.end()); 

     std::copy(songs.begin(), songs.end(), 
        std::ostream_iterator<song>(std::cout, "\n")); 
     return 0; 
} 

コンパイルが、ベクトルのないファイルフォーマット

ONテストされていない(しかし、それらを学ぶしてください)この:

 std::vector<song> songs((std::istream_iterator<song>(file)), 
           std::istream_iterator<song>()); 

は次のように書くことができます。

const size_t sz=20; 
song songs[sz]; 
for(unsigned i=0; i!=sz && file; ++i) 
    file >> songs[i]; 

と府の残りの部分関数呼び出しは、

std::random_shuffle(songs, songs+sz); 

のように動作しますが、今ではベクトルを真剣に(そして他のコンテナに)学習します。配列は基本的にはあなたの仕事のために非難されていると考えられています。なぜなら、ファイルに20以上の要素があると、バッファオーバーランが発生し、悪いことが起こるからです。

http://en.cppreference.com/w/cpp/container/vector

また、あなたはので、RAIIの(ほとんどの場合、あなたはバグを導入する可能性が高い)に明示的に開いたり閉じたファイルを必要としません:

http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

+0

これは非常によく見えますが、残念ながら私は関数呼び出しなどに組み込む方法を知っているので、私はwouldntベクトルを学んだhavent。笑。私はあなたに投票する非常に良い仕事です。 – gamergirl22

+1

この2番目のレッスンは今すぐ!真剣に – 111111

+0

@ gamergirl22もう一度見て、私は余分なビットを追加しました – 111111

3

を使用しようとしています。 (それは、この場合にも同様である。)それはほとんど常にバグのあるコードを生成

試してみてください。アウト臭いポイントとして

while (std::getline(fin, song[num].title) && 
     std::getline(fin, song[num].artist) && 
     fin >> song[num].mem) 
{ 
     num++; 
     fin>>ws; 
} 

、あなたのシャッフルはひどいスタイルが、正しいです。試してみてください:

void shuffle (char choice, Songs *ptr, int num) 
{ 
    if (choice == '4') 
    { 
     std::random_shuffle(ptr, ptr + num);    //shuffle 
    } 
    for (int i = 0; i<num; i++) //test 
    { 
     std::cout << ptr[i].title << ptr[i].artist << ptr[i].mem << "\n"; 
    } 
}