構造体の配列を返す（部分的に動作する）C++

Question

構造体の配列を返して、メインの内容を出力しようとしています。コードをデバッグするときは、return文の直前の行に戻り、正しい内容を保持していることを示します。int、次にstring（playerIDとname）です。 return文が実行されると、配列はmainに返されますが、配列にはplayerIDだけが保持されます。 nameの値はすべて失われています。誰かがこれが起こる理由と可能な解決策を説明することはできますか？さらなる明確化が必要な場合は、私に知らせてください。

#include<iostream> 
#include<fstream> 
#include<cstring> 
#include<string> 
#include<math.h> 

using namespace std; 

struct Player 
{ 
    int playerID; 
    string name; 
    }; 

Player *fillPlayers(); 

int main() 
{ 
Player *myPlayerPointer; 
myPlayerPointer = fillPlayers(); 
return 0; 
} 

Player * fillPlayers() { 
ifstream file1; 
Player * fullPlayerPointer = new Player[244]; 
Player ourPlayers[244]; 
file1.open("Players.txt"); 
if (file1.fail()) { 
    cerr << "file1 did not open"; 
} 

if (file1.is_open()){ 

while (!file1.eof()){ 
    string size; 
    getline(file1, size); 
    for(int i = 0; i < 244; i++){ 
     file1 >> ourPlayers[i].playerID; 
     file1 >> ourPlayers[i].name; 
    } 
} 
file1.close(); 
} 
fullPlayerPointer = &ourPlayers[0]; 
return fullPlayerPointer; 
} 

Answer 1

このコードはCコードによく似ています。 C++では、std::vectorやstd::arrayのようなRAIIコンテナが用意されています。

問題については、配列を返さず、代わりにintへのポインタを返しています。 What is array decaying?をチェックしてください。

http://en.cppreference.com/w/cpp/container/vector
http://en.cppreference.com/w/cpp/container/array（C++> = 11）

Answer 2

一般正しい答えは、それが所有し、ストレージの寿命を管理するだけでなく、能力を追跡し、必要に応じてそれ自体のサイズを変更するのを助けるようstd::vectorを使用することです。しかし、間違っていることを説明するためにfillPlayersを散歩しましょう。

Player * fillPlayers() 
{ 
    ifstream file1; 
    Player * fullPlayerPointer = new Player[244]; 

動的に割り当てられた244 Playerです。ダイナミックストレージは手動で制御された有効期間を持ち、解放されるまで有効です。残念なことに、この割り当てられたストレージは使用されず、後で正しく置き去られません。delete[]この割り当ては「リークされます」。これは悲惨なことですが、これはストレージを割り当てる「正しい」方法ですが、関数が間違って使用しています。

Player ourPlayers[244]; 

静的に割り当てられた変数は、244 Playerです。この変数は、最も近い中括弧のセット内にのみ存在します{}。その後、無効になります。これは、ourPlayersが関数からの戻り時に、そして呼び出し元がそれを利用できるようになる前に無効にされるので、ourPlayersへの参照はこの関数から返されてはならないことを意味します。

file1.open("Players.txt"); 
    if (file1.fail()) 
    { 
     cerr << "file1 did not open"; 
    } 

常に使用する前にテストしてください。したがって、上記はほぼ正しいです。ほぼ同じテストを実行する次のラインによって冗長化されます。これはelseを置くのに適しているかもしれませんが、コードはif (file1.is_open())で読みやすくなり、その後にelseと表示され、開いていなければエラーメッセージが表示されます。

なぜ私はこれを言うのですか？ is_openのプログラマの意図は、より広い用語failよりもはるかに分かりやすいためです。

if (file1.is_open()) 
    { 

     while (!file1.eof()) 

これは、ほとんどの場合、ファイルをループに間違った解決策である理由の詳細については読むWhy is iostream::eof inside a loop condition considered wrong?。

 { 
      string size; 
      getline(file1, size); 

読み取りが正常に行われたかどうかを常に確認してください。さらに、ファイルサイズが読み取られ、その後に続くループで使用するために整数に変換されませんでした。

  for (int i = 0; i < 244; i++) 

このファイルに含まれているエントリの数にかかわらず、常に読み込まれます。ファイルに少なくとも244のエントリがない場合、読み込みは失敗し、読み込みが成功しているかどうかチェックされていないので、ガベージは配列に格納されます。

また、EOFフラグが設定されるまで、再度読み込みを試みるループで囲まれたファイル内の244エントリをループするループが存在することにも注意してください。ほとんどの場合、そのようなループが1つしか必要ありません。

  { 
       file1 >> ourPlayers[i].playerID; 
       file1 >> ourPlayers[i].name; 
      } 
     } 
     file1.close(); 
    } 
    fullPlayerPointer = &ourPlayers[0]; 

以前製動的割り当てへのポインタが一時的割当ourPlayersへのポインタによって上書きされます。長期記憶装置への参照は、範囲外になり無効になるストレージへの参照に置き換えられました。

OPは、短期間のストレージ内のデータを長期間のストレージにコピーすることを意図していた可能性がありますが、残念ながらそれはコンパイラに指示されたものではありません。ファイルを長期保存に直接読み込むほうがずっと便利です。

return fullPlayerPointer; 

関数から戻り、無効な配列を呼び出し元に返します。 }

そこに修正するロット。

Player * fillPlayers() 
{ 
    ifstream file1("Players.txt"); 

    Player * players = nullptr; 
    if (file1.is_open()) 
    { 
     int size; 
     if (file1 >> size) 
     { 
      players = new Player[size]; 
      int index = 0; 
      while (file1 >> players[index].playerID >> players[index].name 
        && index < size) 
      { 
      } 
      // extra brains needed here to handle premature read failure. 
     } 
     file1.close(); 
    } 
    else 
    { 
     cerr << "file1 did not open"; 
    } 

    return players; // only a pointer to the array was returned. How big the 
        // array is and how many items are actually in it is lost 
} 

std::vectorは本当にすごいとなり場所です。ここで

は、上記の問題のすべてが修正されていますが、いくつかのより多くを公開して、非常に単純なアプローチです。それはどれくらいの大きさでどれくらいいっぱいかを知っています。配列はありません。今

、STDを想定した::ベクトルが許可されていない、とポール・マッケンジーはすでにcovered what to do if it isn'tを持って、実行するスマートなものが使用vectorの安全性と使いやすさを提供するいくつかのささやかを取得するために、アレイの周りに非常に単純なラッパーを作るです。

class stupidvector 
{ 
    Player *players; 
    size_t capacity; // a typical vector implementation uses a pointer for 
        // this to make iteration easier 
    size_t size; // vector uses pointer here, too. 
public: 

    stupidvector(); 
    stupidvector(size_t size); 

    // correctly copy a stupid vector Rule of Three. In this case, don't 
    // allow it to be copied. 
    stupidvector(const stupidvector& src)=delete; 

    // correctly move a stupid vector Rule of Five 
    stupidvector(stupidvector && src); 

    // release storage 
    ~stupidvector(); 

    // add an item to the end 
    void push(const Player & player); 

    // how big is it? 
    size_t getcapacity(); 

    // how full is it? 
    size_t getsize(); 

    // make it act like an array 
    Player & operator[](size_t index); 

    // correctly assign. Rule of Three. again we don't want to copy these, 
    // but if we did, look at Copy and Swap Idiom for a neat solution 
    stupidvector& operator=(const stupidvector& src) = delete; 

    // correctly move 
    stupidvector& operator=(stupidvector && src); 
}; 

は適切にインデント/コードをフォーマットしてくださいRules of Three and Five