Overloading problem

[DynamicLink]

I overloaded the << operator for a time class i started making.
The problem is that outputting an object of the class goes fine, but when i try to output t1+t2, as it will be shown in the code, it throws me an error. Can't understand why. the + operator returns an object of type Timp.

Main.cpp

#include "Timp.h"

using namespace std;

const Timp amiaza(12, 0, 0);
const Timp noapte(0, 0, 0);

int main()
{
    Timp t1(10, 50, 10), t2(5, 10, 10);
    cout << t1; // this one is ok

    cout << (t1+t2); // error: no match for 'operator<<'

    return 0;
}

Timp.h

#ifndef TIMP_H
#define TIMP_H
#include <iostream>

using namespace std;

class Timp
{
    public:
        Timp(short = 0, short = 0, short = 0);
        Timp(const Timp&);
        ~Timp();
        void set_timp(short, short, short);
        void set_h(short);
        void set_m(short);
        void set_s(short);

        const short get_h() const;
        const short get_m() const;
        const short get_s() const;

        Timp operator=(Timp);
        Timp operator+(Timp);
        //Timp operator-(Timp);
        friend ostream& operator<<(ostream&, Timp&);

    private:
        short h, m, s;
};

#endif // TIMP_H 

Timp.cpp

#include "Timp.h"

Timp::Timp(short H, short M, short S)
{
    h = H; m = M; s = S;
}

Timp::Timp(const Timp& t)
{
    h = t.h; m = t.m; s = t.s;
}


Timp::~Timp()
{
    //dtor
}

void Timp::set_timp(short H, short M, short S)
{
    h = H; m = M; s = S;
}

void Timp::set_h(short H)
{
    h = H;
}
void Timp::set_m(short M)
{
    m = M;
}
void Timp::set_s(short S)
{
    s = S;
}

const short Timp::get_h() const
{
    return h;
}
const short Timp::get_m() const
{
    return m;
}
const short Timp::get_s() const
{
    return s;
}


/**
Operatori
*/

Timp Timp::operator=(Timp t)
{
    if(this==&t) return *this;
    this->s = t.s;
    this->m = t.m;
    this->h = t.h;
    return *this;
}

Timp Timp::operator+(Timp t)
{
    Timp t3(*this);
    t3.s += t.s;
    if(t3.s >= 60)
    {
        t3.s -= 60;
        t3.m++;
    }
    t3.m += t.m;
    if(t3.m >= 60)
    {
        t3.m -= 60;
        t3.h++;
    }
    t3.h += t.h;
    return t3;
}

ostream& operator<<(ostream& out, Timp& t)
{
    out << t.h << ':' << t.m << ':' << t.s;
    return out;
}

[vlad from moscow]

Change declaration of the operator

ostream& operator<<(ostream& out, Timp& t);


to

ostream& operator<<(ostream& out, const Timp& t);

[pata]

I am not 100% but I think because your trying to use a function outside your class on a member that is private. To display the data you need to make a member function like combine and display. Functions outside a class that use its members must be declared within the class as friend.:

Timp& combine(Timp objecttoadd){ 
	h += objecttoadd.h;
	m += objecttoadd.m;
	s += objecttoadd.s;
	return *this;}

Timp& display(std::ostream& os){
	os << "hour: " << h << std::endl
	<< "minute: " << m << std::endl
	<< "second: " << s << std::endl;
	return *this;}

If you have a function like:

void combine(Timp& objectAddTo, Timp& objeactToAdd){
	objectAddTo += objectToAdd;}

Since it uses private members of your class you must declare as friend in class or they cant access your members, I dont know if you can or how to declare cout as friend...:

friend void combine(Timp& objectAddTo, Timp& objeactToAdd);

[vlad from moscow]

@pata

The problem is that rvalue is bounded with non-const reference.

[pata]

@vlad from moscow, yea but a function that returns the member called as a lvalue allows you to string member functions and I thik is more usefull for user of your header..

TimpObject.combine(Timpobject2).display(cout);//combine reutns lvalue allowing this to be legal

[vlad from moscow]

I do not understand what you are writing. The problem is resolved. Your functions are not needed.

[pata]

If you use a member function to modify your members it makes finding errors much easier. Examples:

Allowing user to use + operator:

TimpObject.h += TimpObject0.h;
TimpObject.m += TimpObject0.h; //user c&p this line but doesnt modify second argument, common error compiler doesnt catch should be TimpObject.m += TimpObject0.m;  

Now your searching both class and user code for error.

If I only allow member functions to modify class members, I know if objects data is not as expected, it is an error in my member function. Also, a function like I posted returns a lvalue which is more usefull for various reasons.

User is forced to use member function to modify member like so:
TimpObject.combine(TimpObject0);

Because function returns lvalue I can do this:

TimpObject.combine(Timpobject0).display(cout);

Which is equivalent to with less typing:

TimpObject.combine(Timpobject0);
TimpObject.display(cout);

So both solutions work but I think my method prevents problems down the road.

[vlad from moscow]

Your functions make the code only more complicated. Nobody knows what means combine. But everybody understands what means operator +. Moreover combine is not a substitution for operator + because its behaviour is different.

[pata]

Doesnt take the validity away that it makes it easier to find the errors, those are example functions and he can name them as he pleases. They were not direct code to c&p, he can call it addAllClassMembers if he wants.

In the book I learned from c++ primer, it teaches you this method for exactly this reason. But yes it makes it slightly more difficult in the sense that you must anticipate the users needs and make a function for them and label it in a way they dont have to look at the class to understand. Also its not clear what + does without looking at the class as you specify its behavior.

[vlad from moscow]

as I said you are making the code more complicated and less intuitively understandable. I do not advice to write such functions as you are suggesting. In my opinion it is a bad style. As I already said nobody knows what means combine.

[pata]

Then why c++ books teach you to do it my way? ;) And the fact you can change the behavior of the + operator makes doing it your way bad for the reason your arguing, you dont really know what + is doing without looking in the class. Rename "combine" to "addAllClassMembersFrom(Timp& object)" add its function becomes apparent.

[vlad from moscow]

I did not read the books you are reading.:) You should not trust all what is written in books.
For example in many books I see snips of code that demonstrates in fact how the code should not be written.

At present I read "C# Concisely" By J. Bishop and N. Horspool and may say that their code examples are written in very bad style.:)

[pata]

Yes but were not talking examples, were talking about fundamentals of writing a class.. Your saying its more complicated my way because the user may have to look at the class to find out exactly what the function does but allowing the + operator does exactly the same thing because you specify its behavior. To be certain of its behavior you must look in class...

You label member functions to imply what they do so the user knows without looking

Timp& addAllMembersFrom (Timp objecttoadd){ 
	h += objecttoadd.h;
	m += objecttoadd.m;
	s += objecttoadd.s;
	return *this;}

Now its crystal clear.

[vlad from moscow]

You are mistaken It is not crystal clear

Timp& addAllMembersFrom (Timp objecttoadd){ 
	h += objecttoadd.h;
	m += objecttoadd.m;
	s += objecttoadd.s;
	return *this;}

Nobody will understand what means addAllMembersFrom. And why should he add all members? He shall not know what private members the class has. So the name addAllMembersFrom has no any sense.
Moreover it has nothing common with operator +. In any case you have to write the operator + that the user could write intuitively clear (as you said crystal clear) code

Timp t1 = t2 + t3;
I understand what this code means and I think everybody also understand. But it is totally unclear what means addAllMembersFrom. I can not even write an equivalent record for the statement I showed above. Something as

Temp t1;

t1.addAllMembersFrom( t2 ).addAllMembersFrom( t3 );

It is an awful code.

[pata]

Here you go straight from the book, your not understanding me, maybe I am not able to explain as well:

Key Concept: Benefits of Encapsulation
Encapsulation provides two important advantages:
C++ Primer, Fifth Edition
• User code cannot inadvertently corrupt the state of an encapsulated object.
• The implementation of an encapsulated class can change over time without
requiring changes in user-level code.
By defining data members as private, the class author is free to make
changes in the data. If the implementation changes, only the class code
needs to be examined to see what effect the change may have. User code
needs to change only when the interface changes. If the data are public,
then any code that used the old data members might be broken. It would be
necessary to locate and rewrite any code that relied on the old representation
before the program could be used again.
Another advantage of making data members private is that the data are
protected from mistakes that users might introduce. If there is a bug that
corrupts an object’s state, the places to look for the bug are localized: Only
code that is part of the implementation could be responsible for the error.
The search for the mistake is limited, greatly easing the problems of
maintenance and program correctness.

If you use the code as you put it it is equal to

Timp t0,t1,t2
t0 += t1;
t0 += t2;

Everyone should know whether their class will add more than two objects together as you are creating how it is used, you can use initialize list for function parameters with the same type and make a function that can take any number of class objects and adds them... Or do you think I wouldnt know if a user of my class would need to add just 2 objects? Cause if they need to add more than two, they need to write there own class cause mine has a specific purpose... If the user needed to add three and I didnt write a function for it, then I designed the class poorly.

[vlad from moscow]

In fact friend functions are parts of a realization of a class. They have access to private members of a class. So if you are changing realization of a class you shall take into account realizations of friend functions.
Of course the author of the topic could write operator << instead of

ostream& operator<<(ostream& out, Timp& t)
{
    out << t.h << ':' << t.m << ':' << t.s;
    return out;
}

something as

ostream& operator<<(ostream& out, const Timp& t)
{
    out << t.get_h() << ':' << t.get_m() << ':' << t.get_s();
    return out;
}

and there is no heed to declare this operator as friend.

However we are speaking about operator + and your proposal to substitute it for some function as addAllMembersFrom. It is a very bad idea.

[vlad from moscow]

t1 + t2

and

t1 += t2;

are different operations.

For example if you want only to display a sum of two objects then you can write

std::cout << t1 + t2;

It is not equivalent to

std::cout << ( t1 += t2 );

because the last statement has a side effect. It changes the state of t1.

[pata]

ok how about this

Class members:

string username;
int logins;
double activetime;

Can I go:

Classname user,user0;

user += user0;

Nope, so how do I add only logins and activetime, I can specify that behavior with the + operator in my class. But does that make sense and is it clear to the user? So you write a member function to do so correctly with no chance of the user screwing things up. I think your used to writing headers only you are going to use...

Members are private so I cant call:

user.logins = user0.logins

Code as you are recommending is poor because of the multiple reasons stated in the quote from c++ primer, the reason for using member functions is the same reasons to use encapsulation... If you disagree with me, you disagree with what users regard as the best c++ book available.

1.User code cannot inadvertently corrupt the state of an encapsulated object.

2.The implementation of an encapsulated class can change over time without
requiring changes in user-level code.

3.If the implementation changes, only the class code needs to be examined to see what effect the change may have. It would be necessary to locate and rewrite any code that relied on the old representation
before the program could be used again.

4. Another advantage of making data members private is that the data are
protected from mistakes that users might introduce. If there is a bug that
corrupts an object’s state, the places to look for the bug are localized: Only
code that is part of the implementation could be responsible for the error.

[vlad from moscow]

You are quoting the book even not understanding what is written in it. About what corruption of the state of an object are you saying? Your example is awful.
Nobody understand what this statement means

user += user0;

As for class Timp it is totally different situation.

Your arguments are totally invalid. You invent some bad examples where it is not clear what the operator + means and when ( I do not understand why) referencing the quote from the book about some corruption of an object.

What is the relation between operator + and corruption of an object?!

[pata]

t1 + t2

and

t1 += t2;

are different operations.

For example if you want only to display a sum of two objects then you can write

std::cout << t1 + t2;

It is not equivalent to

std::cout << ( t1 += t2 );

because the last statement has a side effect. It changes the state of t1.

Yea I didnt say it was... I said thats what my function does. You could easily add a function to display two objects added then displayed without modifying the original objects... As i said earlier, you must anticipate the users needs, you should know if the user needs to do that based on what your class does.

If you want to be correct, then explain how your method also accomplishes what I posted from the c++ Primer and I will change my own methods of writing a class and give you credit on everything I ever write as well as inform anyone of that books error when presented with the oppertunity :)