HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Time (only) class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classonlytime

Problem

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


TimeOnly.cpp

```
#include "TimeOnly.h"
#include
#include

TimeOnly::TimeOnly(int hour, int minute, int second):
hour_(hour),
minute_(minute),
second_(second)
{
if(
(hour_ >= 0 && hour_ = 0 && minute_ = 0 && second_ 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_;
}

Solution

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

Code Snippets

virtual ~TimeOnly();
private:
    int hour_;
    int minute_;
    int second_;
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); }

Context

StackExchange Code Review Q#11052, answer score: 7

Revisions (0)

No revisions yet.