Time provider library using type safe handler - like chrono CPP

Question

I'm trying to develop a c++ library that provide two kind of times:

UTC time(self explainitory)
System time(time since the system has went on)

I have looked into the chrono library in cpp and im trying to implement my library, more of the same like chrono. the implementation is very complicated for me right now.

I'll post the complete code I have later on, but so far I support only UTC time. I have few problems in my code that I want to improve but I don't know how exactly.

#include "atltime.h"
#include "mmsystem.h"

namespace tc{
    typedef struct date{
        unsigned int day;
        unsigned int month;
        unsigned year;

        date(unsigned int _day, unsigned int _month, unsigned int _year)
        {
            day = _day;
            month = _month;
            year = _year;
        }

        date() :day(0), month(0), year(0){}
    }date;


    template <class _Clock,
        class _Time = typename _Clock::time,
            class _Date = typename _Clock::date>
    class time_point
    {
    public:
        typedef _Clock clock;
        typedef _Time time;
        typedef _Date date;

        time_point(const time& otherTime, const date& otherDate)
        {
            _MyTime = otherTime;
            _MyDate = otherDate;
        }

        time get_time() const
        {
            return (_MyTime);
        }

        date get_date() const
        {
            return (_MyDate);
        }

    private:
        _Time _MyTime;
        _Date _MyDate;
    };
}

struct utc_clock
{
    typedef unsigned long long time;
    typedef tc::date date;
    typedef tc::time_point<utc_clock> time_point;
public:
    static time_point now()
    {
        SYSTEMTIME systime;
        GetSystemTime(&systime);

        time_t cur_time = time(0);
        struct tm *time_buf = localtime(&cur_time);

        time t = (((time_buf->tm_sec + (time_buf->tm_min * 60) + (time_buf->tm_hour * 3600)) * 1000) + systime.wMilliseconds) * 1000ULL;

        tc::date d((unsigned int)systime.wDay, (unsigned int)systime.wMonth, (unsigned int)systime.wYear);

        return time_point(t, d);
    }

    static time to_micro(const time_point& t)
    {
        return t.get_time();
    }

    static time to_mili(const time_point& t)
    {
        return to_micro(t) / 1000;
    }

    static time to_sec(const time_point& t)
    {
        return to_mili(t) / 1000;
    }

    static time to_min(const time_point& t)
    {
        return to_sec(t) / 60;
    }

    static time to_hour(const time_point& t)
    {
        return to_min(t) / 60;
    }

    static char * to_utc_time_str(const time_point& t)
    {
        char *str = new char[100];

        sprintf(str, "%02d:%02d:%02d:%03d:%03d", to_hour(t), to_min(t), to_sec(t), to_mili(t), to_micro(t));
        return str;
    }

    static char * to_utc_date_str(const time_point& t)
    {
        char *str = new char[100];

        sprintf(str, "%02d.%02d.%04d", t.get_date().day, t.get_date().month, t.get_date().year);
        return str;
    }
};

The usage goes like that:

int main()
{

    tc::time_point<utc_clock> t = utc_clock::now();

    char * str = utc_clock::to_utc_date_str(t);

    unsigned long long micro, mili, second;
    unsigned long long s;

    micro = utc_clock::to_micro(t);
    mili = utc_clock::to_mili(t);
    second = utc_clock::to_sec(t);

    s = micro + mili;  // this is not good, need to use type-safe 
                       //handler on return time, or have a converter to allow it.

    cout << "Micro: " << micro << endl;
    cout << "Mili: " << mili << endl;
    cout << "Seconds: " << second << endl;

    cout << "UTC Date is: " << str << endl;

    getchar();

}

There are few things I think I can improve in my code, but don't know how to implement it.

all the function in to_micro, to_mili, to_sec ... etc can be written in otherway using operator overloading maybe? for example I can have 'sc' operator that can convert to seconds based on the received time. sc(1000) // 1000 is in mili, will convert it automatically to 1.
I want to make sure that doing arithmetical operation on different kind of times(mili/seconds/micro) is avoided. I'd be happy to get a direction over it.

Any other suggestions/idea will be greatly welcomed

Welcome to code review. Does the code to be reviewed work as expected? You indicate that there are issues, Do these issue affect how the code runs? — pacmaninbw
– pacmaninbw ♦, Commented Nov 22, 2019 at 14:22
@pacmaninbw thank you. The code runs without bugs. I have issued all the problem that might occur and that I want to avoid. which of them was not clear? maybe I need to explain better — nonamer92
– nonamer92, Commented Nov 22, 2019 at 14:56

user673679 · Accepted Answer · 2019-11-22 17:11:17Z

Fix all Warnings and Errors

During development it is a best practice to get all warning messages as well as all error messages. Warning messages can indicate hidden bugs in the code. For most C++ compilers the -wall switch will provide all warnings. I am building using Visual Studio 2019 Professional and there are many warnings presented during the build:

1>------ Build started: Project: timelibrary2, Configuration: Debug Win32 ------
1>timelibrary2.cpp
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): warning C4477: 'sprintf' : format string '%02d' requires an argument of type 'int', but variadic argument 1 has type 'utc_clock::time'
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%lld' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%I64d' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): warning C4477: 'sprintf' : format string '%02d' requires an argument of type 'int', but variadic argument 2 has type 'utc_clock::time'
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%lld' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%I64d' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): warning C4477: 'sprintf' : format string '%02d' requires an argument of type 'int', but variadic argument 3 has type 'utc_clock::time'
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%lld' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%I64d' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): warning C4477: 'sprintf' : format string '%03d' requires an argument of type 'int', but variadic argument 4 has type 'utc_clock::time'
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%lld' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%I64d' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): warning C4477: 'sprintf' : format string '%03d' requires an argument of type 'int', but variadic argument 5 has type 'utc_clock::time'
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%lld' in the format string
1>D:\ProjectsNfwsi\CodeReview\timelibrary2\timelibrary2\timelibrary2.cpp(107,16): message : consider using '%I64d' in the format string
1>timelibrary2.vcxproj -> D:\ProjectsNfwsi\CodeReview\timelibrary2\Debug\timelibrary2.exe
1>Done building project "timelibrary2.vcxproj".
========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========

The above warnings are all on line 107 which is this line:

        sprintf(str, "%02d:%02d:%02d:%03d:%03d", to_hour(t), to_min(t), to_sec(t), to_mili(t), to_micro(t));

There is also a warning in the IDE about this line:

        time t = (((time_buf->tm_sec + (time_buf->tm_min * 60) + (time_buf->tm_hour * 3600)) * 1000) + systime.wMilliseconds) * 1000ULL;

This warning is Arithmetic Overflow, which can indeed create errors or bugs.

Avoid `using namespace std;`

While it is not in the code under review, it is apparent from the code in main() that the using namespace std; is in the code.

If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This stack overflow question discusses this in more detail.

struct utc_clock

In the declaration of struct utc_clock there is the public: declaration, this is not necessary since by default all contents of a struct are public.

It is not clear why all the functions in the struct are declared as static. It means while many instances of the utc_clock struct may be instantiated, they all refer to one instance of each function.

If the utc_clock struct is being use to convert the system up time to utc, it might be good to add a function to_days as well, since some systems may be up for days, weeks or months.

Declare and Initialize Variables at the Same Time

A good practice in C++ is to initialize the variables when they are declared. C++ does not automatically initialize variables as some other languages do. Each declaration and initialization should be in it's own statement, on its own line, this makes it easier to find and edit the variable declaration and initialization.

Instead of

    unsigned long long micro, mili, second;

    micro = utc_clock::to_micro(t);
    mili = utc_clock::to_mili(t);
    second = utc_clock::to_sec(t);

it would be better to declare and initialize like this

    unsigned long long micro = utc_clock::to_micro(t);
    unsigned long long mili = utc_clock::to_mili(t);
    unsigned long long second = utc_clock::to_sec(t);

    unsigned long long s = micro + mili;

Unused Variables

In the main() function, the variable s is declared and assigned a value but it is never referenced. It would better if the variable s wasn't declared or initialized.

Initialization of Private Variables in a Constructor

It is possible and generally desired to private variables in a class outside the body of the constructor. This can be accomplished using the {} operator, so instead of

        time_point(const time& otherTime, const date& otherDate)
        {
            _MyTime = otherTime;
            _MyDate = otherDate;
        }

use

        time_point(const time& otherTime, const date& otherDate)
        : _MyTime{otherTime}, _MyDate{otherDate}
        {
        }

Thank you for your comment. noted everything. what about the type-safety issue regarding the returned type?(to_sec/to_micro etc..) how can I solve this? — nonamer92
– nonamer92, Commented Nov 22, 2019 at 17:24

Stack Exchange Network

Time provider library using type safe handler - like chrono CPP

1 Answer 1

Fix all Warnings and Errors

Avoid `using namespace std;`

struct utc_clock

Declare and Initialize Variables at the Same Time

Unused Variables

Initialization of Private Variables in a Constructor

You must log in to answer this question.

Hot Network Questions

Time provider library using type safe handler - like chrono CPP

1 Answer 1

Fix all Warnings and Errors

Avoid using namespace std;

struct utc_clock

Declare and Initialize Variables at the Same Time

Unused Variables

Initialization of Private Variables in a Constructor

You must log in to answer this question.

Related

Hot Network Questions

Avoid `using namespace std;`