Getting errors trying to pass a vector to a class's function C++

Question

Im trying to read a text file "dictionary.txt" that contains some words with their definition and type. Each word is meant to be loaded into a Word class object with the definition and type, this object is then meant to be pushed to a vector array of other Word objects.

However I'm getting the errors:

E0147 declaration is incompatible with "void Dictionary::loadDictionary(std::vector<<error-type> std::allocator<<error-type>>> &vect)" (declared at line 27)

and

E0020   identifier "loadDictionary" is undefined.

I'm pretty new to C++ and OOP in general so would love some help with these errors.

Thanks heaps!

Code:

#include "stdafx.h"
#include <fstream>
#include <iostream>
#include <string>
#include <vector>
using namespace std;

class Dictionary
{
public:
    void loadDictionary(vector<Word>& vect);

private:
    Word w1;
    string word;
    string def;
    string type;    
};

void Dictionary::loadDictionary(vector<Word>& vect)
{
    ifstream dicFile;
    dicFile.open("dictionary.txt");


    if (!dicFile)
    {
        cout << "File not found!" << endl;
        exit(1);
    }

    int count1 = 0;
    while (!dicFile.eof())
    {
        w1 = new Word;
        dicFile >> word;
        dicFile >> def;
        dicFile >> type;
        w1.word->word;
        w1.def->def;
        w1.type->type;
        vect.push_back(w1);
    }
}
class Word
{
public:

private:
    string word;
    string definition;
    string type;

};

Word::Word() {
    word = "";
    definition = "";
    type = "";
}




int main()
{
    Dictionary d;
    vector<Word> word;
    d.loadDictionary(word);
    return 0;
}

Answer 1

If you push all of code to single file, then just make an object of Dictionary and call to function loadDictionary()

Dictionary d; d.loadDictionary(word);

Answer 2

The code needs to be revised completely. But some quick fixes that are pretty obvious:

• The "new" operator returns pointer to the object. In your code you are storing the pointer into a non-pointer variable of the type "Word".
• Inside the "while" loop you are trying to access a "Word" object inside another "Word" and then accessing private variables of the class inside "loadDictionary" function.
• The data between two classes are duplicated.
• Instead of a class for "Word" you can use "struct" which is simpler, however it doesn't really matter if you use classes correctly.

Answer 3

Here is a set of suggestions to make everything work and start thinking to the problem in a more OOP way (but this may be subjective).

As others pointed out, the main problem is that w1 is a Word and you try to do

w1 = new Word;

This makes no sense, since new Word creates a pointer to a Word (a Word*), which is not what you want. C++ is not Java, in which everything is an implicit pointer to something. Here you can have automatic objects (Word) and pointers to objects (Word*).

From a class design point of view you create a Word which is supposed to keep together the three strings word, definition, and type. Ok. What is a Dictionary? The name suggests it is a container for words, so the vector should be an attribute of the Dictionary, rather than a parameter which gets filled. Otherwise the name should have been DictionaryLoader or something in those lines.

So I'd start by fixing the Word class. To make things simpler I suggest that you have everything public, so I'll use struct instad of class. Following Google C++ Style Guide I added an underscore after the member variable names. Since the initialization is not needed, I'd avoid it. Instead you will load words from a stream, so it may be a nice idea to have a method for loading a word. An operator would be even better, but let's leave it for the future. The way you were reading didn't allow for definitions including spaces. So I took the liberty of using getline to use quoted strings (no quotes inside!).

This is an example dictionary.txt (which you should have included in your question! Remember Minimal, Complete, and Verifiable example):

sovereign "a king or queen" noun
desk "a type of table that you can work at, often one with drawers" noun
build "to make something by putting bricks or other materials together" verb
nice "pleasant, enjoyable, or satisfactory" adjective

And here goes the code.

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

struct Word {
    std::string word_;
    std::string definition_;
    std::string type_;

    std::istream& read(std::istream& is) {
        is >> word_;
        std::string skip;
        std::getline(is, skip, '"');
        std::getline(is, definition_, '"');
        is >> type_;
        return is;
    }
};

Now the Dictionary. A dictionary is a container for words, so our dictionary should have a vector of words inside it. All the variables which were in your dictionary were not really in the right place. You were using them as temporaries, so they should have been placed inside your function.

struct Dictionary {
    std::vector<Word> vect_;

    bool load(const std::string& filename) {
        std::ifstream is("dictionary.txt");
        if (!is)
            return false;

        while (true) {
            // Read
            Word w;
            w.read(is);
            // Check
            if (!is)
                break;
            // Use
            vect_.push_back(w);
        }

        /* Alternative
        Word w;
        while (w.read(is)) { // Read & Check
            // Use
            vect_.push_back(w);
        }*/

        /* Another alternative
        for (Word w; w.read(is);) { // Read & Check
            // Use
            vect_.push_back(w);
        }*/

        return true;
    }
};

int main()
{
    Dictionary d;
    if (d.load("dictionary.txt"))
        return EXIT_SUCCESS;
    else
        return EXIT_FAILURE;
}

Check the Dictionary::load function. The rule is simple: Read, Check, Use. My suggestion is to always start with an infinite loop with the three comments. Then add the relevant code to make the read, then that for checking, and finally use what you just read. Then look for more compact alternatives, if you really need them.

Ah, I just remembered: since you are using VisualStudio, do yourself a favor: don't use precompiled headers. You don't know what they are and, believe me, you won't need them for a very long time. So, create your projects with "Windows Desktop Wizard", don't create a directory for the solution, and in the following dialog select "Empty Project".