Operation Overloading - did I do this properly?

[Immo]

I'm looking at the + overload. Something about it feels off. Would it be better to return *this by instantiating a new object in this case or just return it as it is?

class Time
{
public:
	friend Time operator+(const Time&, const Time&);
	friend std::ostream& operator<<(std::ostream&, const Time&);
private:
	int seconds, minutes, hours;
public:
	Time() = default;
	Time(int h, int m, int s) : hours(h), minutes(m), seconds(s) {}
	
};

Time operator+(const Time& lhs, const Time& rhs)
{
	return Time(lhs.hours + rhs.hours, lhs.minutes + rhs.minutes, lhs.seconds + rhs.seconds);
}

std::ostream& operator<<(std::ostream& out, const Time& rhs)
{
	out << rhs.hours << ":" << rhs.minutes << ":" << rhs.seconds;

	return out;
}

int main()
{
	Time obj1(5, 4, 25), obj2(4, 3, 22);

	cout << "obj1: " << obj1 << endl <<
		"obj2: " << obj2 << endl;

	Time obj3 = obj1 + obj2;

	cout << "obj3: " << obj3 << endl;

}

[CS Student]

When you test it does it work?

[Immo]

Yes, but I wouldn't like to get in the habit of utilizing this approach if it isn't the most efficient.

[JLBorges]

When implementing a class, start by thinking about the class invariant.

When designing a class, it is often useful to consider what's true for every object of the class and at all times. Such a property is called an invariant.
- Stroustrup's' FAQ http://www.stroustrup.com/bs_faq.html#class

The values of the members and the objects referred to by members are collectively called the state of the object (or simply, its value). A major concern of a class design is to get an object into a well defined state (initialization/construction), to maintain a well defined state as operations are performed ... The property that makes the state of an object well-defined is called its invariant.
- Stroustrup in 'The C++ Programming Language'

class Time
{
    int seconds; // invariant [0,59]
    int minutes ; // invariant [0,59]
    int hours; // invariant [0,23]

    bool is_valid() const
    {
        return seconds >= 0 && seconds < 60 &&
               minutes >= 0 && minutes < 60 &&
               hours >= 0 && hours < 24 ;
    }

    public:
        // ...
};

It is the job of every constructor to establish the class invariant, so that every member function can rely on it. Every member function must leave the invariant valid upon exit.
- Stroustrup's' FAQ http://www.stroustrup.com/bs_faq.html#class

Thus, the purpose of initialization is to put an object into a state for which the invariant holds. Typically, this is done by a constructor. Each operation on a class can assume it will find the invariant true on entry and must leave the invariant true on exit. The destructor finally invalidates the invariant by destroying the object.
- Stroustrup in 'The C++ Programming Language'

So, these won't do at all:

public: Time() = default;
Time(int h, int m, int s) : hours(h), minutes(m), seconds(s) {}

Write a default constructor that puts the object into a valid state.
In the constructor that accepts arguments, perform validation, correct the values or report errors..
In either constructor, verify that the class invariant is established as the last step.
For instance, to assist debugging: assert( is_valid() ) ;

You might want to provide public getters (accesssors) and setters (mutators) to allow clients to access the state of the object.

"There are several benefits to be obtained from restricting access to a data structure to an explicitly declared list of functions. For example, any error causing a Date to take on an illegal value (for example, December 36, 2016) must be caused by code in a member function. This implies that the first stage of debugging – localization – is completed before the program is even run. This is a special case of the general observation that any change to the behavior of the type Date can and must be effected by changes to its members.

In particular, if we change the representation of a class, we need only change the member functions to take advantage of the new representation. User code directly depends only on the public interface and need not be rewritten...

Another advantage is that a potential user need examine only the definitions of the member functions in order to learn to use a class. A more subtle, but most significant, advantage is that focusing on the design of a good interface simply leads to better code because thoughts and time otherwise devoted to debugging are expended on concerns related to proper use.

The protection of private data relies on restriction of the use of the class member names."
- Stroustrup in 'The C++ Programming Language 4th Ed'

class Time
{
    // ...
    public:
        // Each operation on a class can assume it will find the invariant true on entry
        int Hours() const { assert( is_valid() ) ; return hours ; } // getter, const

        void Hours( int h ) // setter, so not const
        {
            assert( is_valid() ) ;
            // validate h,
            // correct/throw on error
            // update state
            assert( is_valid() ) ; // Each operation on a class must leave the invariant true on exit.
        }

        // ....
};

Finally, let us look at operator overloading.

> "Something about it feels off"

Does it seem logical to apply a binary plus operation between two objects of type Time?
What should 10:23:42 + 17:03:26 mean?
Do we add time points in real life?

Incrementing time by a certain time interval is more natural. For instance: 10:23:42 + 362 seconds

class Time
{
    // ...
    
    public:
        // Each operation on a class can assume it will find the invariant true on entry
        Time& operator+= ( int secs )
        {
            assert( is_valid() ) ;
            // adjust time by secs seconds
            // secs seconds later if secs is positive
            // secs seconds earlier if secs is negative
            assert( is_valid() ) ;
            return *this ;
        }

        Time& operator-= ( int secs ) { return *this += -secs ; }

    // ...
};

// prefer overloading operators with non-member, non-friend functions

Time operator+ ( Time t, int secs ) // note: t is passed by value
{ return t += secs ; }

Time operator+ ( int secs, Time t ) { return t += secs ; }

Time operator- ( Time t, int secs ) { return t -= secs ; }