Clock class

[Groogy]

Yo!

I'm trying to construct a Clock class based on the functions in ctime.

Though it's not going very well I get some odd behavior with it adding/removing a hour when it shouldn't. So thought I should just pick one from the internet, look at it and see what I did wrong and probably use that one too. But googling got me nowhere.

So I turned here

Well what I need is a clock. Not a Timer, I want it to be able to use/view/read time in this format: YY-MM-DD hh:mm:ss

Thx!

**EDIT** The code if you might spot what I did wrong. I've marked the areas where it goes wrong. So long I have narrowed it down to operator+=

//Header file
class Clock {
    public:
        // Functions removed to make more room!

    private:
        // The time data
        tm * currentTime;
};

//Source file
Clock::Clock() {
    this->currentTime  = Clock::nowTM();
}

Clock::Clock(tm * time) {
    this->currentTime = (tm *) malloc(sizeof(tm));
    memcpy(this->currentTime, time, sizeof(tm));
}

Clock::Clock(const std::string& time) {
    this->currentTime = (tm *) malloc(sizeof(tm));

    strptime(time.c_str(), "%Y-%m-%d %H:%M:%S", this->currentTime);
}

Clock::Clock(const Clock& clock) {
    this->currentTime = (tm *) malloc(sizeof(tm));
    memcpy(this->currentTime, clock.currentTime, sizeof(tm));
}

Clock::~Clock() {
    free(this->currentTime);
}

Clock Clock::operator+(time_t i) const {
    time_t t = mktime(this->currentTime);
    t += i;
    return Clock(localtime(&t));
}

Clock Clock::operator-(time_t i) const {
    time_t t = mktime(this->currentTime);
    t -= i;
    return Clock(localtime(&t));
}

Clock& Clock::operator=(time_t i) {
    memcpy(this->currentTime, localtime(&i), sizeof(tm));
    return *this;
}

Clock& Clock::operator=(const string& time) {
    tm date = *this->currentTime;
    strptime(time.c_str(), "%Y-%m-%d %H:%M:%S", &date);
    memcpy(this->currentTime, &date, sizeof(tm));

    return *this;
}

Clock& Clock::operator=(const Clock& time) {
    this->currentTime = (tm *) malloc(sizeof(tm));
    memcpy(this->currentTime, time.currentTime, sizeof(tm));

    return *this;
}

Clock& Clock::operator+=(time_t i) {
    // Output here will show correct time
    time_t t = mktime(this->currentTime);
    t += i;
    memcpy(this->currentTime, localtime(&t), sizeof(tm));
    // Output here will have decreased time with 1h. If I add +3600 it works
    // the first time but the next time it's 1h in the future
    return *this;
}

Clock& Clock::operator-=(time_t i) {
    Clock c = this->operator-(i);
    memcpy(this->currentTime, c.currentTime, sizeof(tm));
    return *this;
}

bool Clock::operator>(const Clock& clock) const {
    time_t sec1 = this->toSeconds();
    time_t sec2 = clock.toSeconds();

    return sec1 > sec2;
}

bool Clock::operator<(const Clock& clock) const {
    time_t sec1 = this->toSeconds();
    time_t sec2 = clock.toSeconds();

    return sec1 < sec2;
}

bool Clock::operator>=(const Clock& clock) const {
    time_t sec1 = this->toSeconds();
    time_t sec2 = clock.toSeconds();

    return sec1 >= sec2;
}

bool Clock::operator<=(const Clock& clock) const {
    time_t sec1 = this->toSeconds();
    time_t sec2 = clock.toSeconds();

    return sec1 <= sec2;
}

string Clock::toDefaultString() const {
    char buffer[512];
    strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", this->currentTime);

    return string(buffer);
}

string Clock::toHourTime() const {
    char buffer[4];
    strftime(buffer, sizeof(buffer), "%H", this->currentTime);

    return string(buffer);
}

string Clock::toMinuteTime() const {
    char buffer[4];
    strftime(buffer, sizeof(buffer), "%M", this->currentTime);

    return string(buffer);
}

string Clock::toSecondTime() const {
    char buffer[4];
    strftime(buffer, sizeof(buffer), "%S", this->currentTime);

    return string(buffer);
}

time_t Clock::toSeconds() const {
    return mktime(this->currentTime);
}

tm * Clock::nowTM() {
    time_t secs; time(&secs);
    return Clock::getTM(secs);
}

tm * Clock::getTM(time_t t) {
    tm * timeraw = localtime(&t);
    tm * timecpy = (tm *) malloc(sizeof(tm));
    memcpy(timecpy, timeraw, sizeof(tm));
    return timecpy;
}

[WingedPanther]

There are several things that I'm finding odd about your code.

1) You are using the this pointer A LOT. Why?
2) You haven't listed the necessary ctime include in your code. Are there any other includes we need to know about for this code?
3) Do you have a testing program that is causing you to think the += operator is functioning incorrectly, as opposed to anything else?

[Groogy]

There are several things that I'm finding odd about your code.

1) You are using the this pointer A LOT. Why?
2) You haven't listed the necessary ctime include in your code. Are there any other includes we need to know about for this code?
3) Do you have a testing program that is causing you to think the += operator is functioning incorrectly, as opposed to anything else?

1) Bad habit? I like to really see that I am working on "this" object.
2) Well I thought they weren't required but they are: ctime, cstdlib, cstring, sstream, string
3) Yeah:

// Engine retrieves data from a database. The last update value is written in the format mentioned previously
cout << engine.getLastUpdate().getValue() << endl;
Clock lastUpdate = engine.getLastUpdate().getValue(); lastUpdate += 1;
cout << "waiting: " << lastUpdate.toDefaultString() << endl;

When lastUpdate += 1 is removed it works. But I need to add by one, or I'll be updating the same second forever and ever.

[ZekeDragon]

@WingedPanther:
1) Why shouldn't he use the this pointer? Using this makes it clear where the variable/method comes from, since there's no other indication as to where to find it. I don't know groovy's programming experience, but doing that is a common Java idiom.
2) I would say he omitted it for clarity. He did the same with the class methods themselves on the header.

As far as the problem is concerned:

    t += i;

You don't add the two values of time together, that's your problem I suspect. This will cause one really big number to be added to another really big number. Instead, you want to add the difference between the two to t.

    t += (i - t);

I may be wrong about that, but I think that's your problem. Give it a shot.

EDIT: Just saw your post, I'm probably wrong then. >_< I'll look it over.

Could we get a raw copy of the compilable code? Just the Clock class and the test condition is fine, I just want to be able to build it and test it on this machine, so I can toy with it.

[Groogy]

Aight, I gave you a part of the main.cpp file. I've taken away the parts concerning the engine (cause other wise your computer would start connecting to my laptop here )

If you need more let me know.

**EDIT** How yeah, don't forget to edit the text string with a date in it to something much closer to now. Or else the program will work for quite a long time

***EDITAGAIN***
Well forgot to include Clock.hpp and uncomment a line with engine in main.cpp, but shouldn't be a problem for you to find and fix yourself.

Anyway a funny thing I noticed here now, it seems to work in this limited example except that I am adding 3600 seconds on purpose.. So gonna try and find what difference I've made.

Aight, I've tried replicating it, got a screenshot from the result which is the same as before... don't get... WHY????

[ZekeDragon]

Yeah, I was actually going to mention that this works fine when you remove +3600. Perhaps there is something else that is causing a problem.

Does this cause a problem in your for loop? IE does removing adding the single second consistently remove an hour from the clock?

[Groogy]

This is making me nuts. Look at the screenshot, I've modified it to replicate the example... But the problem still persist? What do I have in the original that can interfere?

And don't mind the for-loop. I have set a break-point before the for-loop...
But actually, when it comes to the for-loop it works like it should (except it's -3600 from what it should be)

It's that initial +1 second that seems to screw it up.

**EDIT**
Tried removing the initial +1, but the copy between lastUpdate to currentTick puts it back 3600 seconds....

I'm trying out old fashioned commenting out stuff I got it working now, so am slowly uncommenting other parts until I find it.

[ZekeDragon]

Okay, try using this in your += operator:

Clock& Clock::operator+=(time_t i) {
    std::cout << this->currentTime->tm_isdst << ", " << this->currentTime->tm_hour << "\n";
    time_t t = mktime(this->currentTime);
    t += i;
    memcpy(this->currentTime, localtime(&t), sizeof(tm));
    std::cout << this->currentTime->tm_isdst << ", " << this->currentTime->tm_hour << "\n";
    return *this;
}

I think this might have something to do with DST, though I'm not quite sure how yet...

EDIT: All right, good luck.

[Groogy]

Got that son of a penguin... It was this line:

// Retrieve the server data.
engine.retrieveServerData();

It's right before the part where I start working with the clock
Why? I have no idea.

SmartPointer<Nation> Engine::getNations() {
    if(this->nations.getSize() > 0)
        return this->nations;

    this->nations = Nation::findAllWhere("1=1");
    return this->nations;
}

void Engine::retrieveServerData() {
    this->getNations();
    for(unsigned long i = 0; i < this->nations.getSize(); i++) {
        Nation * nation = this->nations.getPointer() + i;

        // Cache data
        nation->getShips(); nation->getMODs(); nation->getBases();
    }
}

Nothing concerning the clock in there : I'm starting to lose it....




**EDIT**

EUREKA! Could it be that mysql++ reconfigure the time for the process or something so that I get UTC time for my process instead of +1h as I get normally? Because that's the only possibility I can see really.

*Nah just remembered, "now" is still correct time for the clock...

[WingedPanther]

Here's my thought: you've got a lot of stuff going on, with multiple different classes interacting. What you should probably do is create a file called "testclock.cpp" that can be used to verify the functionality of each overloaded operator, constructor, etc. Once you've done that, you can be certain that a bug either is or is not in your Clock class, and focus testing on bad use of it.