Confused about using constructurs with istream parameters

Question

I have a slight problem when trying to learn how to create constructors.

I am currently learning c++ using the "C++ Primer" book and I've come to a point where I am told to create some constructors then change a code using these constructors. The exercise states that I should rewrite this program using the istream constructor but I don't know how to do this.

int main()
{
    Sales_data total; 
    if (read(cin,total))
    { 
        Sales_data trans; 
        while (read(cin,trans))
        {
            if (total.isbn() == trans.isbn())
            {
                total.combine(trans);
            }
            else
            {
                print(cout, total) << endl;
                total = trans;
            }
        }
        print(cout, total) << endl;
    }
    else
    {
        cerr << "No data?!" << endl;
    }    
    return 0;
}

The problem I have is, I have no idea how I am supposed to use an constructor using the istream, i thought it would be simple and just pass in cin as a default value but it does not work. from visual studios I get a "LNK2019" error and from code::blocks "undefined reference to Sales_data::read(std::istream&, Sales_data&)

My code in my header file looks like this:

struct Sales_data 
{
    Sales_data() = default;
    Sales_data(const std::string &s) : bookNo(s){}
    Sales_data(const std::string &s, unsigned n, double p) :
                    bookNo(s), units_sold(n), revenue(p*n){}
    Sales_data(std::istream &is) 
    {
         read(is, *this); 
    }

    std::string isbn() const { return bookNo; };
    Sales_data& combine(const Sales_data&);
    double avg_price() const;
    Sales_data add(Sales_data&, Sales_data&);
    std::ostream &print(std::ostream&, const Sales_data&);
    std::istream &read(std::istream&, Sales_data&);

    std::string bookNo;
    unsigned units_sold = 0;
    double revenue = 0.0;
};

plus some definitions below.

and my cpp file looks like this:

int main()
{
    Sales_data total(cin); //results in error "LNK2019" or "undefined reference to Sales_data::read(std::istream&, Sales_data&)"

    if (1)
    { //not really sure what to use here but if I get my default value to work I might figure it out.
        // I'm thinking it should work with just cin >> total or read(total)
        Sales_data trans(cin); //results in error "LNK2019" or "undefined reference to         Sales_data::read(std::istream&, Sales_data&)"
        while (1)
        {
            if (total.isbn() == trans.isbn())
            {
                total.combine(trans);
            }
            else
            {
                print(cout, total) << endl;
                total = trans;
            }
        }
        print(cout, total) << endl;
    }
    else
    {
        cerr << "No data?!" << endl;
    }

    return 0;
}

I hope you understand my problem, and I appreciate all the help you provide! :)

You need to implement the function. I don't see it implemented in your posted code. — R Sahu, Jun 08 '14 at 05:34
Why are you passing `cin`? Why don't you use Class instead of struct, it would make your code more readable! — Samer, Jun 08 '14 at 05:35
This is how far I've in the book, and the book uses struct so therefore I'm using it as well, and the reason I'm passing cin is that I don't really know the meaning behind constructors otherwise. I want it to become read(cin,total) somehow. before in my if statements i used Read(cin, total) to read from the istream but the excercise stated that I should rewrite that program to use the istream constructor which I frankly don't know how to use. *Updated so it contains the original code as well. — Tim0n, Jun 08 '14 at 05:41
r-sahu answered the question. Additional comments: The function `read` does not have a function body in the definition of `Sales_data`. It is only declared there. You need to say somewhere what `read` really does. If `Sales_data` is defined in a file `Sales.h` then a fitting place for the definition of `std::istream &Sales_data::read(std::istream&, Sales_data&){...}` is a file `Sales.cpp`. The separation of declaration and definition avoids problems if you include `Sales.h` in multiple translation units of your project. You need to compile `Sales.cpp` and link the output to your executable. — Tobias, Jun 08 '14 at 08:44
@r-sahu Please write your comment as and answer so that the OP can accept it. Please consider to include my remarks in your answer. Thanks in advance. — Tobias, Jun 08 '14 at 08:46
@samer The only difference between `class` and `struct` is that `class` starts implicitly with `private` while `struct` starts implicitly with `public`. The keyword `struct` is also used for class definitions in the `g++` headers and in the standard draft where appropriate. So I consider it as best practice. E.g., it does not make sense to use `class` when a class only has public members. You only need to keep in mind that you have to search for the regular expression `(class|struct) Foo` if you search for the definition of the class `Foo`. Just to give the idea.The RE can be more complicated. — Tobias, Jun 08 '14 at 09:04
Thank you I read your answers and found the error, I had forgotten to use the Sales_data:: scope in my read function, it had been working properly before but with the combination of constructors it broke the code it is now fixed and is working. — Tim0n, Jun 08 '14 at 13:17

score 1 · Accepted Answer · answered Jun 08 '14 at 08:48

1

It sounds to me as if you're either missing some code from your book, or the book is expecting you to implement other functions and not just the constructor.

The linker error is telling you that it's unable to find an implementation of your read function, which should look like:

std::istream& Sales_data::read(std::istream&, Sales_data&)
{
    // TODO - implementation here.
}

It's also worth mentioning that function implementations should be added to the source (.cpp) file.

answered Jun 08 '14 at 08:48

Ben Cottrell

5,741
1
27
34

Oh wow, so the error was that I did not include Sales_data:: in the function my function looked like this std::istream &read(std::istream&, Sales_data&) { // TODO - implementation here. } adding the Sales_data:: scope fixed it, now I just have to figure out how to use my read statement in an if() statement, which apparently became illegal. But thank you for your help! – Tim0n Jun 08 '14 at 13:13
If you replace the definition `read(...)` by `Sales_data::read(...)` then you must also replace the call of `read(...)` by `total.read(...)`. Here I assume that `total` is the relevant object of type `Sales_data`. Maybe, you do not need the argument of type `Sales_data&` then? If you want to keep the argument you can actually define a friend function `read(...)` instead of `Sales_data::read(...)`? This is also good practice. If you like the encapsulation into `Sales_data` for some reason you could also use a static member function instead of a member function. – Tobias Jun 08 '14 at 13:49
@timOn Sorry, I forgot to add that the previous comment was for you. – Tobias Jun 08 '14 at 14:10

Confused about using constructurs with istream parameters

1 Answers1