0

one of our assignments in the class is to create a program that uses objects to display hours, minutes and seconds. With these numbers we now have to overload various operators that will increase the seconds/minutes by 1, and decrease them using ++ and --. Im having some trouble with the -- operator as its not working as expected, if I put in 0 minutes, and it decreases the minutes it returns values like 128 minutes. As Im starting out in this I would really appreciate some help.

And then the second part is using other operators (> < >= <= == !=) to compare 2 different hours, minutes and seconds and return a bool value if one is more than the other (i.e. h:1 m:5 s:0 vs h:0 m:5 s:0 will return 'true'). I havent gotten to the second part and would appreciate some pointers on how to get this started as Im trying to wrap my head around this whole idea logically.

Main.cpp

#include <iostream>
#include "Time.h"

using namespace std;

int main() {

    int hour1, minute1, second1, hour2, minute2, second2;

    cout << "Enter time A (hh, mm, ss): ";
    cin >> hour1;
    cin >> minute1;
    cin >> second1;
    cout <<endl;

    /*cout << "Enter time B(hh, mm, ss): ";
    cin >> hour2;
    cin >> minute2;
    cin >> second;
    cout <<endl;*/

   Time T1(hour1, minute1, second1);

   ++T1;                    //Increases seconds by 1
   T1.displayTime();

   T1++;                    //Increases minutes by 1
   T1.displayTime();

   --T1;                    //Decreases seconds by 1
   T1.displayTime();

   T1--;                    //Decreases minutes by 1
   T1.displayTime();


   return 0;
}

Time.h

#ifndef TIME_H
#define TIME_H

class Time
{
    public:
        Time();
        Time (int h, int m, int s);

        void displayTime();

        Time operator++();
        Time operator++(int);

        Time operator--();
        Time operator--(int);
        /*Time operator>();
        Time operator<();
        Time operator>=();
        Time operator<=();
        Time operator==();
        Time operator!=();*/

    private:
        int hours;
        int minutes;
        int seconds;
};

#endif // TIME_H

Time.cpp

#include <iostream>
#include "Time.h"

using namespace std;

Time::Time(){
    hours = 0;
    minutes = 0;
    seconds = 0;
}

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

void Time::displayTime(){
    cout << "Hours: " << hours <<" Minutes: " << minutes << " Seconds: " <<seconds <<endl;
}

Time Time::operator++(){ //Prefix plus seconds
    ++seconds;
    if (minutes >= 60){
        ++hours;
        minutes -= 60;
    }
    if (seconds >= 60){
        ++minutes;
        seconds -= 60;
    }
    return Time(hours, minutes, seconds);
}

Time Time::operator++(int){ //Postfix plus minutes
    Time T(hours, minutes, seconds);

    ++minutes;
    if(minutes >=60){
        ++hours;
        minutes -= 60;
    }
    if (seconds >= 60){
        ++minutes;
        seconds -= 60;
    }
    return T;
}

Time Time::operator--(){ //PREFIX MINUSS seconds
    --seconds;

    if (seconds == 0){
        --minutes;
        seconds += 59;
    }

    if (minutes == 0){
        --hours;
        minutes += 59;
    }

    return Time(hours, minutes, seconds);
}

Time Time::operator--(int){ //POSTFIX MINUSS minutes
    Time T(hours, minutes, seconds);
    --minutes;

    if (minutes == 0){
        --hours;
        minutes += 59;
    }
    if (seconds == 0){
        --minutes;
        seconds += 59;
    }
    return T;
}


/*Time Time::operator>(){

}

Time Time::operator<(){

}

Time Time::operator>=(){

}

Time Time::operator<=(){

}

Time Time::operator==(){

}

Time Time::operator!=(){

}
*/

If you spot any other mistakes, please do let me know.

So with this the minutes are not subtracting correctly. It seems that it is just taking away from 0, but not adding the necessary seconds back to it (if that makes sense).

Thank you.

B. Baxter
  • 133
  • 1
  • 10
  • 1
    Please read about [mcve] and please remove the parts of the code that are commented out anyhow. This looks like a huge wall of code, when actually it isnt that much – 463035818_is_not_an_ai Apr 16 '19 at 10:00
  • As @user463035818 said, your example may be complete, but it is far from minimal. – Fantastic Mr Fox Apr 16 '19 at 10:00
  • You wrote `Time::Time(int h, int m, int s){ hours = h; minutes = m; seconds = s; }` - this is better (uses the initialization list) : `Time::Time(int h, int m, int s) : hours(h), minutes(m), seconds(s) { }` – Jesper Juhl Apr 16 '19 at 10:01
  • the way you check if seconds or minutes exceed `60` is far from optimal. Use divsion, modulus and correct ordering (seconds first, than minutes etc). – Stack Danny Apr 16 '19 at 10:02
  • You should first increment minutes before incrementing hours. – Aconcagua Apr 16 '19 at 10:05
  • Post-increment increments minutes only??? That's not the usual meaning of 'post-increment'. Usually: `Time tmp(*this); ++*this; return tmp;` – Aconcagua Apr 16 '19 at 10:07
  • `#ifndef TIME_H` is rather generic as a header guard name and likely to clash with other stuff in a large project. A better header guard name would be something like `PROJECTNAME_MODULENAME_TIME_H`. See also https://stackoverflow.com/a/49688652/5910058 – Jesper Juhl Apr 16 '19 at 10:09
  • Usually, prefix operator returns current object, so that you could do ++(++time): `Time& operator++() { /*increment*/ return *this; }` – Aconcagua Apr 16 '19 at 10:10
  • `--seconds; if(seconds == 0)` – nope: 0 is a valid value, and you can decrement it once more. So either check before (`if(seconds-- == 0)` or check afterwards for negative result (`if(seconds == -1)` or `if(seconds < 0)`). – Aconcagua Apr 16 '19 at 10:13
  • I'd suggest removing the implementation of the default constructor and instead do `Time() = default;` in the class declaration and initilize the members in-class, like `private: int hours = 0; int minutes = 0; int seconds = 0;` . – Jesper Juhl Apr 16 '19 at 10:19
  • About [using namespace std](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)... – Aconcagua Apr 16 '19 at 11:46

1 Answers1

2

Im having some trouble with the -- operator

Actually, you have quite some more trouble! Already operator++ won't work as expected. Try:

Time t(0, 59, 59);
++t;

Once you incremented seconds with overflow, next that can overflow is the minutes, so you need to check these first!

++seconds;
if(seconds == 60)
{
    seconds = 0;
    ++minutes;
    // only, if minutes were incremented, they can overflow, so check only here needed
   if(minutes == 60)
   {
       minutes = 0;
       // open: how do you want to handle hours overflowing?
       // variant 1: just go on, counting 23, 24, 25, ...
       ++hours;
       // variant 2: restart at 0:
       hours = (hours + 1) % 24;
       // variant 3 (my favourite): rember in a flag that we overflowed
       // (and have a getter for so that user can check):
       isWrapAround = hours == 23; // new member variable of type bool
       hours = (hours + 1) % 24;
   }
}

Analogously, you'd handle the operator--, just replacing every occurence of ++ with -- and adjusting the overflow detection to underflow detection. Careful with the latter: Your original code did not do proper underflow detection:

--seconds;
if(seconds == 0)

This would already decrement minutes when we actually have 1 second left, but 00:00:00 is a valid time! So you need to either check for 0 before decrementing (if(seconds-- == 0) or check for negative values afterwards (--seconds; if(seconds == -1) or if (seconds < 0)). With this fix, += 59 won't be correct any more either, you'd need either += 60 or preferrably simply = 59.

Usually, the pre-increment and -decrement operators return a reference to current object. This would allow for e. g. ++(++time):

Time& Time::operator++()
{
    // increment as described
    return *this;
}

The post-increment and -decrement operators are very strange... Please re-validate if it really is the task to in-/decrement minutes (if so, I can only shake heads over your teacher...). It will be a great surprise for anyone as the operators behave totally differently from what is the usual behaviour! The latter one would be:

Time operator++(int)
{
    Time tmp(*this);
    ++*this;
    return tmp;
}

If you really, really shall increment minutes (note the irony: post-X-crement operators actually behave like pre-X-crement, at least as your initial approach looks like): Just leave seconds untouched. All you then need in the post-operators is the body of the outermost if in the respective pre-variants. These then could be re-written (to avoid code duplication) as:

++seconds;
if(seconds == 60)
{
    seconds = 0;
    *this++; // use post-fix to adjust minutes...
}

Finally: comparison: Unfortunately, we don't yet have C++20 available, otherwise we simply could have implemented the spaceship operator (<=>)... Never mind, we still can use an ordinary function instead and use this one in the operators:

int Time::compare(Time const& other) const
{
    // most relevant are hours, if these differ, values of minutes or
    // seconds don't matter any more...
    int result = hours - other.hours;
    if(result == 0)
    {
        // so hours are the same...
        // minutes then are relevant next
        result = minutes - other.minutes;
        if(result == 0)
            result = seconds - other.seconds;
    }
    return result;
}

Then all the operators to be implemented would look like:

bool Time::operator@(Time const& other) const
{
    return compare(other) @ 0;
}

where @ stands for all of your operators needed (==, !=, <, <=, >, >=).


Bonus: remembering overflow without separate flag:

You won't need another variable any more, but it requires more complex logic.

At first, leaving the -1 (operator--) will indicate that a wrap-around occured. Correspondingly, the getter for wrap-around would return hours == -1;.

At any other place where we previously used hours directly, we'd now use the getter for, which would look like this:

int Time::getHours() const
{
    return hours + (hours == -1);
}

Calculating the value for increment is slightly more complex:

hours = (getHours() + 1) % 24 - (hours == 23);
Aconcagua
  • 24,880
  • 4
  • 34
  • 59
  • This was really helpful. I think my ++ and -- work now, did several runs and all seems well. Working on the comparisons now, struggling a bit but I will get there. Thanks! – B. Baxter Apr 16 '19 at 19:42
  • @Acancogua In the comparison (>) what would that look like in main.cpp? `Time T2 (hour2, minute2, second2); //Creating the second object T1>T2;` Or am I way off here? – B. Baxter Apr 16 '19 at 19:54
  • @B.Baxter Exactly: `Time t1(h1, m1, s1), t2(h2, m2, s2); if(t2 > t1)...;`; if you use pointers, you need to de-reference (otherwise, pointer addresses would be compared): `Time* t1 = ...; Time* t2 = ...; if(*t2 > *t1)...`. – Aconcagua Apr 17 '19 at 07:50
  • Actually, you could compare a single object with itself, too: `Time t1(h, m, s); if(t1 < t1)` – provided the operator is correctly implemented in the usual meaning, this should always yield a constant value no matter with which values initialised, only depending on the operator type used. Sole exception: if floating point comparison is involved and the value is `NaN`, as `NaN` (to be more precise: any of the several representations) compares unequal to *anything*, even itself... – Aconcagua Apr 17 '19 at 07:56