Implementing a Time class in c++

Design a class TIME which stores hour, minute and second. The class
should have


the methods to support the following:



-
User may give the time value in 24-hour format.

-
User may give the time value in AM/PM format

-
Display the time in 24-hour format.

-
Display the time in AM/PM format.

-
User may like to add minute with a time value.

#include 
#include 

class Time {
    public:
        Time(int h = 0, int m = 0, int s = 0, const std::string &am_or_pm = "");
        void print_time24();
        void print_time12();
        void add_min();
    private:
        int hr;
        int min;
        int sec;
};

Time::Time(int h, int m, int s, const std::string &am_or_pm):
    hr(h), min(m), sec(s)
{
    if (am_or_pm == "AM" && hr == 12)
        hr = 0;
    if (am_or_pm == "PM" && hr != 12)
        hr += 12;
}

void Time::print_time24()
{
    std::cout > op;
    cout > hr >> min >> sec;
    string am_or_pm;
    if (op == 1)
        cin >> am_or_pm;
    Time t(hr, min, sec, am_or_pm);
    t.print_time12();
    cout << '\n';
    t.print_time24();
    cout << '\n';

    cout << "Adding a minute,\n";
    t.add_min();
    t.print_time12();
    cout << '\n';
    t.print_time24();
    cout << '\n';

    return 0;
}

The code works but can it be made better? Also what will be the correct way to handle invalid arguments in the constructor?

I see some things that may help you improve your program.

Use const where practical

The print_time24() and print_time12() functions only print and do not modify the underlying Time object, so they should both be declared const:

void print_time24() const;
void print_time12() const;

Make printing prettier

The current print_time24() routine does not emit nice looking time. If the time is 12:06:07, the program emits 12:6:7 which is not quite right. Instead, you have to format the data so that minutes and seconds are both two digits. I show how in the next comment.

Create a stream inserter

Right now the code is only capable of printing to std::cout and not a std::stringstream or std::ofstream. This can be fixed by one of two ways. Either you can add a std::ostream& parameter to each of the print routines or you can create a friend inserter like this:

friend std::ostream& operator<<(std::ostream& out, const Time& t) {
    auto oldfill = out.fill('0');
    return out << t.hr 
        << ':' << std::setw(2) << t.min 
        << ':' << std::setw(2) << t.sec << std::setfill(oldfill);
}

You will also have to #include . Now it can be used like so:

cout << t << '\n';

Think of the user

While the add_min() routine does indeed add exactly 1 minute, I would instead have expected that one would be able to add an arbitrary number of minutes instead, as with add_min(int minutes).

Also, it seems likely that a user would have a preference for either AM/PM or 24-hour time. I'd suggest making that preference either a boolean data member of the class or perhaps a static data member for all instances of the class.

Consider signed vs. unsigned

Would a time of -3:-80:-155 be something you would recognize as a time? Probably not, but unfortunately your Time object has no problem with that. Both the constructor arguments and the private data members should be unsigned rather than int. If you want to save some space, you could even declare the private data members as unsigned char.

Use braces

Lines like this:

if (am_or_pm == "AM" && hr == 12)
    hr = 0;

while technically accurate are a potential error waiting to happen. The problem is that if someone adds a line, perhaps for troubleshooting or diagnostics, the if statement will break unless braces are added. You can both make your code more clear and prevent this kind of future error by using braces.

Handle out-of-range constructor arguments

There are two choices for how to handle invalid constructor arguments. Either you can ignore them and silently substitute a default or throw an error. I'd recommend the latter approach. At the bottom of your constructor, add these three lines:

if (hr > 24 || min > 59 || sec > 59) {
    throw(std::domain_error("Time value out of range"));
}

You will also have to #include .

Use helper functions to make code simpler

The code to print the time could be much simpler if you use helper functions. I'd suggest these member functions:

bool is_pm() const {
        return hr >= 12;
    }
private:
    unsigned get_hour(bool am_pm) const {
        if (!am_pm)
            return hr;
        if (is_pm())
            return hr-12;
        if (hr == 0)
            return 12;
        return hr;
    }
    const char *get_suffix(bool am_pm) const {
        if (!am_pm)
            return "";
        if (is_pm())
            return " PM";
        return " AM";
    }

Now the two separate print routines can be refactored as one member function taking a bool to tell whether to print 24-hour or AM/PM time:

std::ostream& print(std::ostream &out, bool am_pm) const
{
    auto oldfill = out.fill('0');
    return out << get_hour(am_pm)
        << ':' << std::setw(2) << min 
        << ':' << std::setw(2) << sec 
        << get_suffix(am_pm)
        << std::setfill(oldfill);
}

This, in turn can be used by the friend inserter I mentioned above:

friend std::ostream& operator<<(std::ostream& out, const Time& t) {
    return t.print(out, false);
}

Simplify the add_min() code

The code does more mathematics than it needs to. Now that the constructor throws an error if it's malformed, we know that the stored values are always valid. This means that the add_min() code can be simplified:

void Time::add_min()
{
    if (++min > 60) {
        hr = (hr + 1) % 24;
        min = 0;
    }
}

Eliminate return 0 at the end of main

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.

StackExchange Code Review Q#118130, answer score: 3