今、あなたのコードはやりすぎです。プログラムは、より大きな問題を解決するために、一連のサブ問題を解決します。これは、単一責任の原則につながります。
つまり、1つのオブジェクト（クラス、関数など）は1つの問題のみを解決する必要があります。しかし、今はそれは起こっていません。たとえば、main自明に複数のことを行います。リストのノードを管理し（誤って、何も削除されません！）、ユーザーからの入力を取得します。これは、やりすぎ。
むしろ、物事を分割します。ノードを管理するlistクラスを作成してから、それmainを使用する必要があります。ここでの違いに注意してください。mainもはやその問題を解決するのではなく、解決する何かを利用します。
したがって、これを念頭に置いて、物事を分割すればするほど、修正、修正、および保守が容易になります。コードを取得して分割するという行為は「リファクタリング」です。そうしよう。
まず、使用するリンクリストが必要です。通常、私たちはstd::vector（注：リンクリストは一般的に悪いコンテナです）またはstd::listを持っていますが、あなたの先生はばかげているので、あなたにあなた自身を書かせています。割り当ては、リストコンテナーを作成するか、リストコンテナーを使用して入力を読み取るか、両方ではない必要があります。（繰り返しますが、現実の世界では物事を分割します。なぜ人々にそれらを混ぜるように教えるのですか？）
あなたはすでに基本を理解しています、それはただカプセル化される必要があります。（クラスをまだ知らない場合は、私に知らせてください。私もそこで展開します。私たちがそれをしている間、まだ知らない場合は、先生が何であるかを自分自身に教えるための良い本を手に入れたいと思うかもしれません。 t）：
// normally this should be a template so it can store anything,
// and yadda yadda (more features), but let's just make it basic
// this data class is what the linked list holds
struct data
{
std::string name;
int id;
};
class linked_list
{
public:
linked_list() :
mHead(0)
{}
// the magic: the destructor will always run
// on objects that aren't dynamically allocated,
// so we're guaranteed our resources will be
// released properly
~linked_list()
{
// walk through the list, free each node
while (mHead)
{
node* toDelete = mHead; // store current head
mHead = mHead->next; // move to next node
delete toDelete; // delete old head
}
}
void push_back(const data& pData)
{
// allocate the new node
node* newNode = new node(pData, mHead);
// insert
mHead = newNode;
}
data pop_back()
{
// remove
node* oldNode = mHead;
mHead = mHead->next;
// deallocate
data d = oldNode->data;
delete oldNode;
return d;
/*
the above is *not* safe. if copying the data throws
an exception, we will leak the node. better would be
to use auto_ptr like this:
// now the node will be deleted when the function ends, always
std::auto_ptr<node> n(oldNode);
// copy and return, or copy and throw; either way is safe
return n->data;
but who knows if your <strike>dumb</strike>misguided
would allow it. so for now, make it unsafe. i doubt
he'd notice anyway.
*/
}
private:
// any class that manages memory (i.e., has a destructor) also needs to
// properly handle copying and assignment.
// this is known as The Rule of Three; for now we just make the class
// noncopyable, so we don't deal with those issues.
linked_list(const linked_list&); // private and not defined means it
linked_list& operator=(const linked_list&); // cannot be copied or assigned
struct node
{
// for convenience, give it a constructor
node(const data& pData, node* pNext) :
d(pData),
next(pNext)
{}
data d; // data we store
node* next; // and the next node
};
node* mHead; // head of list
};
これで、使用するリストができました。mainそのようなことに悩まされることはもうありません：
#include <cstdlib>
#include <iostream>
#include <fstream>
#include <string>
using namespace std; // should generally be avoided
// your linked_list code
int main()
{
// don't declare variables until you need them,
// and avoid globals. (the previous rule helps)
ifstream sourcefile("A3.txt");
// check that it opened
if (!sourceFile.is_open())
{
cerr << "could not open file" << endl;
// EXIT_FAILURE is inside <cstdlib>
return EXIT_FAILURE;
}
string input;
sourcefile >> input;
cout << "Received command: " << input << endl;
linked_list datalist;
if (input == "%INSERT")
{
string reader;
sourcefile >> reader;
cout << "Insert name: " << reader << endl;
while (reader[0] != '%')
{
data d;
d.name = reader;
sourcefile >> d.id;
datalist.push_back(d);
sourcefile >> reader;
cout << "reader: " << reader << endl;
}
}
}
読みやすいことに注意してください。リストを管理するのではなく、単にそれを使用します。そして、リストはそれ自体を管理するので、何もリークすることはありません。
これがあなたがたどりたいルートです：1つの問題を正しく解決する作業オブジェクトに物事を包み、それらを一緒に使用します。