ファイルの内容を構造体に追加する

Question

私のコレクション内の書籍にインデックスを付けるプログラムを作成することになっています。構造体には、タイトル、著者、出版者などの通常の書籍情報が含まれています。しかし、出力は得られません。 1つの問題は、タイトルにスペースが含まれることです。

/* Book Inventory assignment 2 by Heath Martens. */ 

#include <iostream> 
#include <stdlib.h> 
#include <fstream> 
#include <cstring> // I had to throw this in in order to get memcpy to work. 

using namespace std; 

typedef struct book{ 
    char title[100]; 
    char author[100]; 
    char publisher[100]; 
    float price; 
    int isbn; 
    int pages; 
    int copies; 
} Book; 

Book collection[100]; 
int currentIndex; 


void 
indexBook(Book *my_book) 
{ 
    memcpy(&collection[currentIndex], my_book, sizeof(Book)); 
    currentIndex++; 
} 

void 
readfile(void) 
{ 
    fstream my_stream; 
    string line = " "; 
    my_stream.open("input.txt"); 
    int i=0; 
    for (i = 0; i < currentIndex; i++) 
     { 
      while (getline(my_stream, line)) 
      { 
       cin >> line >> collection[i].title; 
       cin >> line >> collection[i].author; 
       cin >> line >> collection[i].publisher; 
       cin >> line >> collection[i].price; 
       cin >> line >> collection[i].isbn; 
       cin >> line >> collection[i].pages; 
       cin >> line >> collection[i].copies; 
      } 
     } 

    my_stream.close(); 

} 

void 
printCollection(void) 
{ 
    int i; 
    for (i = 0; i < currentIndex; i++) 
    { 
     cout << "Title: " << "\t\t" << collection[i].title << endl; 
     cout << "Author: " << "\t" << collection[i].author << endl; 
     cout << "Publisher: " << "\t" << collection[i].publisher << endl; 
     cout << "Price: " << "\t\t" << collection[i].price << endl; 
     cout << "ISBN: " << "\t\t" << collection[i].isbn << endl; 
     cout << "Pages: " << "\t\t" << collection[i].pages << endl; 
     cout << "Copies: " << "\t" << collection[i].copies << endl; 
    } 
} 

void printCollection(void); 

int 
main(void) 
{ 
    currentIndex = 0; 

    Book *my_book = new Book; 

    indexBook(my_book); 

    readfile(); 

    printCollection(); 

    delete my_book; 

    return 0; 
} 

ここに私が使用するように与えられたtxtファイルがあります。

Magician: Apprentice 
Raymond E. Feist 
Spectra (January 1, 1994) 
5.02 
0553564943 
512 
1 
Magician: Master 
Raymond E. Feist 
Spectra (January 1, 1994) 
7.99 
0553564935 
499 
1 

ここに提供されているいくつかの例に基づいて更新されたコードがあります。

void 
    readfile(void) 
    { 
     fstream my_stream ("input.txt"); 
     if(!my_stream) 
    { 
     return; 
    } 
    string line = " "; 
    int i=0; 
for (i = 0; i < currentIndex; i++) 
    { 
     if(!std::getline(my_stream, line)) 
     { 
      break; 
     } 
     memcpy(collection[currentIndex].title, line.c_str(), std::min(sizeof(collection[currentIndex].title), line.size())); 
     memcpy(collection[currentIndex].author, line.c_str(), std::min(sizeof(collection[currentIndex].author), line.size())); 
     memcpy(collection[currentIndex].publisher, line.c_str(), std::min(sizeof(collection[currentIndex].publisher), line.size())); 
     my_stream >> collection[currentIndex].price; 
     my_stream >> collection[currentIndex].isbn; 
     my_stream >> collection[currentIndex].pages; 
     my_stream >> collection[currentIndex].copies; 
     my_stream.ignore(); 
     if(!my_stream) 
     { 
      break; 
     } 
    } 

Error in output

Answer 1

主な問題は、のReadFile関数です。コメントで述べたように、std :: fstreamは最初の引数として文字列を取ることができるので、openを後で呼び出す必要はありません。さらに、操作を実行する前に、ファイルが開いていることを確認する必要があります。 std :: stringはこの関数の後半で使用するために初期化する必要はありません。これにより、次のコードが生成されます。

fstream my_stream("input.txt"); 
if(!my_stream) { 
    return; 
} 
string line; 

読み取りファイルのループが正しく構築されていないようです。外側のループはコレクション配列の現在のインデックスの範囲を増分し、内側のループはファイル全体を読み込むように見えます。内部ループが正しく構築された場合、ファイルの内容はコレクションの0番目のBookに書き込まれます。内部ループは、std :: cinからの入力を読み込むことによって無視される、ファイルからの行の読み取りが成功したかどうかをテストすることから始まります。

input.txt内の "Books"の数が分かっていないため、外側のループはすべての "Books"を別のコレクション要素に読み込むという目的とは無関係です。ファイル全体を読み込むために、ループを変更して、ファイルが読み込み可能であることを確認します。

while(my_stream) { 

文字列を読み込むために、私たちは対応に行の内容をコピーする（スペースでアップつまずいてしまいますここmy_stream >> line;）読みとstd :: getlineのを使用して行全体を格納する必要がある、とのmemcpy char配列std :: getlineは失敗する可能性があるため、行の内容を使用する前に成功を確認します。例えば、ここでのタイトル：番号について

// title 
if(!std::getline(my_stream, line)) { 
    break; 
} 
memcpy(collection[currentIndex].title, line.c_str(), std::min(sizeof(collection[currentIndex].title), line.size())); 

は、私たちは演算子を使用して数字を>>読み、行の末尾に余分な文字を無視することができます。同様に、この操作は失敗し、同様にチェックされます。例えば、ここでは価格です：現在のブックのすべてのメンバーが正しく読み込まれた場合は

my_stream >> collection[currentIndex].price; 
    my_stream.ignore(); 
    if(!my_stream) { 
     break; 
    } 

、その後、ループ本体の最後に、我々は見書籍（++currentIndex;）の数をインクリメントします。コメントに記載されているように、my_streamの明示的な終了は、my_streamのデストラクタがスコープの最後に呼び出されたときにファイルが閉じられるため、readfileのコンテキストでは必要ありません。

いくつか追加のマイナーポイント。上記のコメントで説明したように、std :: stringは、おそらくBook :: title、Book :: author、Book :: publisherに使用する必要があります。これは、std :: stringがメモリを明示的に管理することなく文字数が不明確な場合を処理するためです。同様に、コレクションは標準コンテナ（std :: vectorなど）に適しています。これは、Bookのコピーコンストラクタを使用してコレクションに格納するように変更できる、現在のindexBookの実装に問題を引き起こします。ブックIOの場合、演算子>>および演算子< <は、readfileおよびprintCollection内のコードを置き換えて単純化するためにオーバーロードされる可能性があります。

Answer 2

このコードでは、多くの問題と習慣があります。

using namespace std; 

それはstdライブラリの名前空間で宣言された名前のすべてが、将来的にあなたに奇妙なあらゆる種類の問題が発生しますグローバル名前空間にインポートされることを意味するので、これは、危険な練習です。

あなたはstd::coutとstd::stringのすべての時間を入力する必要がないようにしたい場合は、代わりに書くことができます。

using std::string; 
using std::cout; 

次へ：

typedef struct book { ... } Book 

これは完全に不要であるC言語の構造ですC++で。

struct Book { 
}; 

これは簡単です。

struct Book { 
    char title[100]; 
}; 

あなたはstd :: stringを認識しているので、ここでそれを使用しないでください。

using std::string; 

struct Book { 
    string title; 
    string author; 
    string publisher; 
    float price; 
    int isbn; 
    int pages; 
    int copies; 
}; 

indexBookの機能は率直に怖いです。

void 
indexBook(Book *my_book) 
{ 
    memcpy(&collection[currentIndex], my_book, sizeof(Book)); 
    currentIndex++; 
} 

memcpyはC++では避けるべきである、あなたの代わりに、組み込みのC++オブジェクトにコピー/割り当て/移動事業者に依存している必要があります。

collection[currentIndex] = *my_book; 

そこにオブジェクトがコピーされない理由は特に良い理由があり、かつオブジェクトのクラスの作者が何か良いですmemcpyであなただけの未定義の動作を取得するのに対し、あなたは、コンパイルエラーになります場合。

少し先をスキップします：

void 
printCollection(void) 
{ 
    ... 
} 

void printCollection(void); 

はここに害はありませんが、それの定義の後、前方宣言printCollectionは冗長です。

Book *my_book = new Book; 

indexBook(my_book); 

... 
delete my_book; 

ここではこれを行った理由は不明です。無駄に思える。あなたは同じように簡単に行っている可能性：

Book my_book; 

しかし、単純な問題があるいずれかの方法：my_bookが初期化されていませんが。コレクションにコピーしたものは誰でも推測できます。

Book my_book {}; 

デフォルトで初期化されます。

ここでreadfileを見てみましょう。

fstream my_stream; 
string line = " "; // why? 
my_stream.open("input.txt"); 
int i=0; 
for (i = 0; ...) 
{ 
    ... 
} 
my_stream.close(); 

ここでは、多くの不必要な作業とコーディングが行われています。オブジェクトにコンストラクタがあるようにC++を使用している場合は、std::string line = " "またはstd::string line(" ")またはstd::string line {" "}（C++ 11より優先）を記述できます。しかし、fstreamでも同じことができます。最後に、iがforループの外側に表示されない限り、ループに対してローカルにすることができます。

fstreamはオブジェクトであるため、デストラクタもあり、ファイルが確実に閉じられます。

このすべては、今、私たちは実際にこの機能にあるほとんどがあなたのコード、と壊れているものの核心に来ている

void 
readfile() 
{ 
    std::fstream my_stream("input.txt"); 
    for (int i = 0; i < currentIndex; ++i) 
    { 
     std::string line {}; // default initialized. 
     ... 
    } 
    // call to close is redundant 
} 

に私たちをダウンさせます。

1あなたは、ファイルはあなたがcurrentIndex

を調整していない

3、forループ内部i whileループでgetline(my_stream, line)を呼び出す

2開いていることをチェックしません。 4（最悪の場合）operator>>を悪用する。 currentIndexが1である、

for (i = 0; i < currentIndex; ++i) // *1 
{ 
    while (getline(my_stream, line)) // *2 
    { 
     cin >> line >> collection[i].title; // *3 

あなたがメインで初期化されていないオブジェクトにindexBookと呼ばれているので、私たちは、forループを入力します：あなたのコードで

は、次のような記述します。

my_streamの最初の行をline（* 2）に読み込み、それが成功するとwhileループ本体に入ります。

今、私たちは、[0] .TITLE（：I = 0、および< currentIndexなければならない* 3は、覚えて）試してみて、コレクションにstd::cinから別の単語に続くラインにstd::cinから単語をお読みください。

これらのチェックは行われません。いずれかが失敗した場合は、何もしないで処理を進めます。

最後に、whileループの最後に到達し、再びgetline(my_stream, line)を試します。これが成功すると、whileループ本体に再入力します。

* iはまだ変更されていないので、最初のパスでからフェッチされたデータを上書きするコードは、引き続きcollection[0]を参照します。

これは、my_streamを使い果たしてから、やっとwhileループを終了し、forループに到達するまで続きます。

iは、iが今度は1になるようにインクリメントされます。条件がチェックされ、i < currentIndexということはもう真ではないので、forループを終了します。

をからcollection[i].titleに読み込むコードを作成していると思われます。

bool 
readFile(void) // returns true on success, false on error 
{ 
    using std::getline; 

    std::fstream my_stream("input.txt"); 
    if (!my_stream.is_open()) 
     return false; 
    for (int i = 0; i < currentIndex; ++i) 
    { 
     Book newBook {}; // temporary local to read into 
     if (!getline(my_stream, newBook.title)) 
      break; 
     if (!getline(my_stream, newBook.author)) 
      break; 
     if (!getline(my_stream, newBook.publisher)) 
      break; 
     std::string line; 
     if (!getline(my_stream, line)) 
      break; 
     newBook.price = std::stof(line); 
     if (!getline(my_stream, line)) 
      break; 
     newBook.isbn = std::stoi(line); 
     if (!getline(my_stream, line)) 
      break; 
     newBook.pages = std::stoi(line); 
     if (!getline(my_stream, line)) 
      break; 
     newBook.copies = std::stoi(line); 

     collection[i] = newBook; 
    } 

    return true; 
} 

これは確かに遠く慣用のC++であることからですが、それはあなたを教えるためにあなたの先生/書籍のためです：私たちは、私たちがこのような何かを書くかもしれませんfloatとintegerに文字列を変換するためにstd::stofとstd::stoiを使用することができます。しかし、私はこの課題をどのように解決するかを先読みします。

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

using std::string; 

struct Book 
{ 
    // suffix member variables with `_` to distinguish from function names 
    // and parameters. 
    string title_; 
    string author_; 
    string publisher_; 
    float price_; 
    int isbn_; 
    int pages_; 
    int copies_; 

    // support `stream >> book` syntax with `operator >>()`. 
    // note that it's not a member of the class, so we declare 
    // it as a `friend` function so it can have full access. 
    friend std::istream& operator >> (std::istream&, Book&); 

    // because of the formatting, I wouldn't make this `operator <<`. 
    void print() const 
    { 
     std::cout << "Title: " << "\t\t" << title_ << '\n'; 
     std::cout << "Author: " << "\t" << author_ << '\n'; 
     std::cout << "Publisher: " << "\t" << publisher_ << '\n'; 
     std::cout << "Price: " << "\t\t" << price_ << '\n'; 
     std::cout << "ISBN: " << "\t\t" << isbn_ << '\n'; 
     std::cout << "Pages: " << "\t\t" << pages_ << '\n'; 
     std::cout << "Copies: " << "\t" << copies_ << '\n'; 
    } 
}; 

using Collection = std::vector<Book>; 

std::istream& operator >> (std::istream& str, Book& book) 
{ 
    std::getline(str, book.title_); 
    std::getline(str, book.author_); 
    std::getline(str, book.publisher_); 
    str >> book.price_ >> book.isbn_ >> book.pages_ >> book.copies_; 
    str.ignore(); 
    return str; 
} 

bool 
readFile(std::string filename, Collection& collection) 
{ 
    std::fstream my_stream(filename); 
    if (!my_stream.is_open()) 
     return false; 

    Book newBook {}; 
    while (my_stream >> newBook) 
     collection.emplace_back(std::move(newBook)); 

    return true; 
} 

void 
printCollection(const Collection& collection) 
{ 
    for (auto&& book : collection) // or: const Book& book 
    { 
     book.print(); 
     std::cout << '\n'; 
    } 
} 

int 
main() 
{ 
    Collection collection; 

    if (!readFile("input.txt", collection)) 
    { 
     std::cerr << "readFile on input.txt failed\n"; 
     return 1; // non-zero return from main indicates program failure 
    } 

    std::cout << "Read " << collection.size() << " books\n\n"; 

    printCollection(collection); 
}