Time (only) class

Question

Working in C++ and I have some code which I'm refactoring. I have some code that needs to work with times of day but not dates. I do some juggling with time_t and struct tm but it's a pain and not intuitive to use. So I decided to make a basic time class with no date component. Let me know if you see any issues or have ideas for improvement:

TimeOnly.h

#pragma once

class TimeOnly
{
public:
    TimeOnly(int hour, int minute, int second);
    virtual ~TimeOnly();

    static TimeOnly GetCurrentTime();
    virtual int GetHour() const;
    virtual int GetMinute() const;
    virtual int GetSecond() const;

private:
    int hour_;
    int minute_;
    int second_;

};

inline bool operator==(const TimeOnly& lhs, const TimeOnly& rhs)
{
    /* do actual comparison */
    return (
        (lhs.GetHour() == rhs.GetHour())
        && (lhs.GetMinute() == rhs.GetMinute())
        && (lhs.GetSecond() == rhs.GetSecond())
        );
} 

inline bool operator!=(const TimeOnly& lhs, const TimeOnly& rhs){ return !operator==(lhs, rhs); } 
inline bool operator<(const TimeOnly& lhs, const TimeOnly& rhs)
{ 
    /* do actual comparison */ 
    if(lhs.GetHour() >= rhs.GetHour())
    {
        return false;
    }
    else if(lhs.GetMinute() >= rhs.GetMinute())
    {
        return false;
    }
    else if(lhs.GetSecond() >= rhs.GetSecond())
    {
        return false;
    }
    else
    {
        return true;
    }
} 

inline bool operator>(const TimeOnly& lhs, const TimeOnly& rhs){ return operator<(rhs, lhs); } 
inline bool operator<=(const TimeOnly& lhs, const TimeOnly& rhs){ return !operator>(lhs, rhs); } 
inline bool operator>=(const TimeOnly& lhs, const TimeOnly& rhs){ return !operator<(lhs, rhs); }

TimeOnly.cpp

#include "TimeOnly.h"
#include <stdexcept>
#include <ctime>

TimeOnly::TimeOnly(int hour, int minute, int second):
    hour_(hour),
    minute_(minute),
    second_(second)
{
    if(
        (hour_ >= 0 && hour_ <= 23)
        && (minute_ >= 0 && minute_ <= 59)
        && (second_ >= 0 && second_ <= 59)
        )
    {
        //ok
    }
    else
    {
        throw std::invalid_argument("Time must be between 0:00:00 and 23:59:59");
    }
}

TimeOnly TimeOnly::GetCurrentTime()
{
    // get time now
    time_t t = time(0);
    struct tm * now = localtime(&t);
    return TimeOnly(now->tm_hour, now->tm_min, now->tm_sec);
}

TimeOnly::~TimeOnly()
{
}


int TimeOnly::GetHour() const
{
    return hour_;
}

int TimeOnly::GetMinute() const
{
    return minute_;
}

int TimeOnly::GetSecond() const
{
    return second_;
}

Edit

This is my revised class taking some suggestions from the answers:

Time.h

#pragma once

class TimeOnly
{
public:
    TimeOnly(int seconds);
    TimeOnly(int hour, int minute, int second);
    ~TimeOnly();

    TimeOnly AddSeconds(int seconds);
    static TimeOnly Now();
    int GetHour() const;
    int GetMinute() const;
    int GetSecond() const;
    int GetTotalSeconds() const;

private:
    void Init(int seconds);
    int secondsInDay_;

    static const int SECONDS_IN_A_DAY = 86400;
    static const int SECONDS_IN_AN_HOUR = 3600;
    static const int SECONDS_IN_A_MINUTE = 60;
};

inline bool operator==(const TimeOnly& lhs, const TimeOnly& rhs)
{
    /* do actual comparison */
    return lhs.GetTotalSeconds() == rhs.GetTotalSeconds();
} 

inline bool operator!=(const TimeOnly& lhs, const TimeOnly& rhs){ return !operator==(lhs, rhs); } 
inline bool operator<(const TimeOnly& lhs, const TimeOnly& rhs)
{ 
    /* do actual comparison */ 
    return lhs.GetTotalSeconds() < rhs.GetTotalSeconds();
} 

inline bool operator>(const TimeOnly& lhs, const TimeOnly& rhs){ return operator<(rhs, lhs); } 
inline bool operator<=(const TimeOnly& lhs, const TimeOnly& rhs){ return !operator>(lhs, rhs); } 
inline bool operator>=(const TimeOnly& lhs, const TimeOnly& rhs){ return !operator<(lhs, rhs); }

TimeOnly.cpp

#include "TimeOnly.h"
#include <stdexcept>
#include <ctime>

TimeOnly::TimeOnly(int seconds)
{
    Init(seconds);
}

TimeOnly::TimeOnly(int hour, int minute, int second)
{
    int seconds = (hour * SECONDS_IN_AN_HOUR) + (minute * SECONDS_IN_A_MINUTE) + seconds;
    Init(seconds);
}


TimeOnly::~TimeOnly()
{
}

void TimeOnly::Init(int seconds)
{
    if(seconds >= 0 && seconds < SECONDS_IN_A_DAY)
    {
        secondsInDay_ = seconds;
    }
    else
    {
        throw std::invalid_argument("Time must be between 0:00:00 and 23:59:59");
    }
}


TimeOnly TimeOnly::AddSeconds(int seconds)
{
    int newSeconds = (secondsInDay_ + seconds) % SECONDS_IN_A_DAY;
    return TimeOnly(newSeconds);
}


TimeOnly TimeOnly::Now()
{
    // get time now
    time_t t = time(0);
    struct tm * now = localtime( & t );
    return TimeOnly(now->tm_hour, now->tm_min, now->tm_sec);
}


int TimeOnly::GetHour() const
{
    return secondsInDay_ / SECONDS_IN_AN_HOUR;
}

int TimeOnly::GetMinute() const
{
    return (secondsInDay_ % (SECONDS_IN_AN_HOUR)) / SECONDS_IN_A_MINUTE;
}

int TimeOnly::GetSecond() const
{
    return secondsInDay_ % SECONDS_IN_A_MINUTE;
}

int TimeOnly::GetTotalSeconds() const
{
    return secondsInDay_;
}

(Note renamed GetCurrentTime() to Now() because I was getting error LNK2001: unresolved external symbol... GetTickCount(void) even though I have no GetTickCount method. Turns out winbase.h has #define GetCurrentTime() GetTickCount())

Why not call it TimeSpan which is the time span since last midnight. This has got to be a solved problem. For instance, see this document for inspiration msdn.microsoft.com/en-us/library/system.timespan.aspx — Leonid, Apr 20 '12 at 21:05
To me, a TimeSpan is conceptually something different. Yes I see how you could use it the same but it also allows arbitrarily large TimeSpans whereas I'm looking for time of day in the 24 range of a day. If it is solved elsewhere I'm all ears. Good idea, however, for looking at TimeSpan for additional methods to add. — User, Apr 20 '12 at 21:21
I will look for the C++ implementations. Looks to me like TimeOnly can wrap (contain) a general TimeSpan and add a few restrictions. — Leonid, Apr 20 '12 at 21:39
I serached online using koders.com/…;* (the google code search alternative), and found something like this: koders.com/cpp/… — Leonid, Apr 20 '12 at 21:46
Same with github.com/… search: github.com/DrFrankenstein/amnlib/blob/… github.com/riviera/plex/blob/… — Leonid, Apr 20 '12 at 21:51

Loki Astari · Accepted Answer · 2012-04-22 23:54:45Z

up vote 7 down vote accepted

Personally I would look at boost. They probably have something.

Comments:

virtual ~TimeOnly();

Are you really going to derive from this class?

Implementation

private:
    int hour_;
    int minute_;
    int second_;

Holding these as separate fields makes the rest of the code more complex.
I personally would hold this a single value (seconds since start of day).

Then all your comparisons become trivial.
Your get functions become slightly more complex but not not very much:

private:
   int secondsInDay;

inline bool TimeOnly::equal(const TimeOnly& rhs)
{
    return secondsInDay == rhs.secondsInDay;
} 

inline bool TimeOnly::less(const TimeOnly& rhs)
{ 
    return secondsInDay < rhs.secondsInDay;
} 

int TimeOnly::GetHour() const
{
    return secondsInDay / (60 * 60);
}

int TimeOnly::GetMinute() const
{
    return (secondsInDay % (60 * 60)) / 60;
}

int TimeOnly::GetSecond() const
{
    return secondsInDay % 60;
}

inline bool operator==(const TimeOnly& lhs, const TimeOnly& rhs) {return lhs.equal(rhs);} 
inline bool operator!=(const TimeOnly& lhs, const TimeOnly& rhs) {return !lhs == rhs); } 
inline bool operator< (const TimeOnly& lhs, const TimeOnly& rhs) {return lhs.less(rhs);} 
inline bool operator> (const TimeOnly& lhs, const TimeOnly& rhs) {return  (rhs < lhs); } 
inline bool operator<=(const TimeOnly& lhs, const TimeOnly& rhs) {return !(lhs > rhs); } 
inline bool operator>=(const TimeOnly& lhs, const TimeOnly& rhs) {return !(lhs < rhs); }

edited Apr 22 '12 at 23:54

answered Apr 20 '12 at 22:23

Loki Astari
23.9k21866

I looked at Boost.Date_Time but as far as I can tell there will always be a date associated. I guess I could use boost's time_duration as the underlying implementation to my class. Probably cleaner, but I thought the class was pretty simple to begin with. Good idea with the seconds of the day. – User Apr 20 '12 at 22:48

Good point about the virtual, you're right this will probably be a "leaf" class. Actually I realized I'll probably need at least an AddSeconds() method, and this is where your data representation really shines, then it's just adding x number of seconds (and modding by the number of seconds in a day to handle the wrap around). – User Apr 21 '12 at 22:11

In your comparison operator definitions, can I access private members even though the operator definitions are non-member functions? – User Apr 21 '12 at 23:08

Yes. Because they are the same class you can access private members of other instances. See: stackoverflow.com/a/437507/14065 – Loki Astari Apr 22 '12 at 18:36

But these are non-member operator definitions, do they need to be declared friend for that to work? – User Apr 22 '12 at 23:41

show 1 more comment

rahul · Answer 2 · 2012-04-21 19:26:08Z

Nicer to split this into a new method (can be reused.)

bool TimeOnly::isVallid(int h, int m, int s) {
    return ((hour_ >= 0 && hour_ <= 23)
     && (minute_ >= 0 && minute_ <= 59)
     && (second_ >= 0 || second_ <= 59)
     );
}

Generally nicer to check validity first in the condition rather than in else.

TimeOnly::TimeOnly(int hour, int minute, int second):
    hour_(hour),
    minute_(minute),
    second_(second)
{
    if(!isValid(hour,minute,second))
        throw std::invalid_argument("Time must be between 0:00:00 and 23:59:59");
}

Think how you would like to keep the seconds minutes hours, Keeping it as seconds as in previous comment is a valid option. Another that does not require you to recompute it but will still simplify your code is to keep it as an array of 3 and index it by enum. Note that it can be extended easily for date, month year too.

enum Time_t
{
    Sec,
    Min,
    Hour
}

You can also use this for your accessors.

int TimeOnly::GetTime(Time_t t) const
{
    return time_[t];
}

This pays off in comparison operators.

inline bool operator==(const TimeOnly& lhs, const TimeOnly& rhs)
{
    /* do actual comparison */
    for(Time_t t = Hour; t >= Sec; t = static_cast<Time_t>(t-1))
       if(lhs.GetTime(t) != rhs.GetTime(t)) return false;
    return true;
}

Or you could use the STL to lexicographically compare both. (The operator becomes a friend.)

bool cmp(int i, int j) {
  return i < j;
}

bool operator<(const TimeOnly& lhs, const TimeOnly& rhs)
{
  return lexicographical_compare(lhs.time_, lhs.time_+3, rhs.time_, rhs.time_+3, cmp);
}

Correspondingly the equality is just

bool operator==(const TimeOnly& lhs, const TimeOnly& rhs)
{
  return equal(lhs.time_, lhs.time_ +3, rhs.time_);
}

This does not look right.

inline bool operator>(const TimeOnly& lhs, const TimeOnly& rhs){ return operator<(rhs, lhs); }

You can directly use the operator rather than use the function syntax.

inline bool operator!=(const TimeOnly& lhs, const TimeOnly& rhs){return !(lhs == rhs);} 
inline bool operator>(const TimeOnly& lhs, const TimeOnly& rhs){return !((lhs < rhs) || (lhs == rhs));} 
inline bool operator<=(const TimeOnly& lhs, const TimeOnly& rhs){return !(lhs > rhs);} 
inline bool operator>=(const TimeOnly& lhs, const TimeOnly& rhs){return !(lhs < rhs); }

Thanks for the suggestions. I like the idea of packaging the validation check into an IsValid function. Realistically I don't think I'd call it anywhere else but it does improve the readability of the code. Using an array with an enum and for loops is interesting but it does not seem clearer/easier to me so I don't think I'd make that change. As for using the operator directly rather than the function syntax in the operator definitions, I'm not sure, I just cut and pasted the code from this Operator overloading question on stackoverflow. — User, Apr 21 '12 at 18:20
Well, The problem is really that of lexicographically ordering a 3-tuple. So I felt it didn't make sense to unroll it. The bonus is that lex-compare is in STL so we can take advantage of that. See my updated version. — rahul, Apr 21 '12 at 19:15

Ryan Leckey · Answer 3 · 2012-04-23 09:44:28Z

Your (modified) solution looks decent. My only noteworthy comments would be to use unsigned instead of int (where appropriate); and, to modify it slightly to not have the '1 day only' restriction.

However, I would recommend that you use std::chrono::time_point or std::chrono::duration from <chrono>. This is a new C++11 header supported in GCC (from 4.6 I believe) and Microsoft Visual C++ (from 11.0); and, the bulk of this proposal came from boost.

Some examples:

#include <chrono>
#include <thread>

// Getting the current time (there is also high_resolution_clock available)
std::chrono::time_point<std::chrono::system_clock> now = 
    std::chrono::system_clock::now();

// Do long time-consuming tasks, sleep for 10 seconds
std::this_thread::sleep_for(std::chrono::seconds(10));

// Get time now
auto later = std::chrono::system_clock::now();

// Is now eariler than later ?
if (now < later)
{
    // Display difference between the two points of time
    auto x = later - now;

    std::cout 
        << std::chrono::duration_cast<std::chrono::seconds>(x).count() 
        << " seconds" << std::endl
        << std::chrono::duration_cast<std::chrono::milliseconds>(x).count() 
        << " milliseconds" << std::endl;
}

Hopefully it will work with what you are trying to do. If anything, look inside the <chrono> header and use it internally. It'll make you never want to look at any code using or pretending to use <ctime>.

Unfortunately using Visual Studio 2010 so don't have full c++11 support — User, Apr 23 '12 at 21:05
Then I would refer you to the second paragraph of my answer. Boost contains a drop-in replacement for your use (until such time that you decide to upgrade your compiler) here. — Ryan Leckey, Apr 24 '12 at 1:15

asked	2 years ago
viewed	373 times
active	2 years ago

current community

your communities

more stack exchange communities

Time (only) class

3 Answers

Comments:

Implementation

Your Answer

Not the answer you're looking for? Browse other questions tagged c++ datetime or ask your own question.

Hot Network Questions

current community

your communities

more stack exchange communities

Time (only) class

3 Answers

Comments:

Implementation

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c++ datetime or ask your own question.

Related

Hot Network Questions