gigasecond: Addresses: 319

[DrJPizzle]

Use a direct implementation of construct_date.
This avoids issues with mktime().

timegm() not used as not universal.

All tests pass with standard solution.

[ryanplusplus]

Hey @DrJPizzle

Thanks for the submission. I'm a bit concerned about the complexity of this solution given that it's much more complicated than the actual problem. If you were to use mk_time instead as you suggested could this be simplified?

Additionally, be sure to run indent.sh prior to committing in order to get the CI build to pass.

[DrJPizzle]

@ryanplusplus mktime was the cause of the problem. It works is 99.9% of cases, but its implementation is pretty complex, (it deals with leap seconds, has complex handling of DST etc) and issues can arise, as seen in the initial issue report.
Normally this is not a problem. In fact, in most places "real" the answer to this problem is complex, and depends on where you are etc and mktime gives you the best guess. However, your tests don't accommodate this, and so if you use mktime, your tests will fail every once in a while.

[ryanplusplus]

@DrJPizzle I was referring to this comment from you:

Note: if using GMT is not fine, and you need the time_t to be correct for your local time, this can be fixed by offsetting with mk_time of a zeroed out struct tm. I can make this change if someone wants.

[DrJPizzle]

This would be an addition. You would still need the existing code.

Perhaps some clarification of thinking is in order:
The gigasecond thing is a hard problem. As a measure of this: there are standards committees that discuss the finer points of the problem. The solution was sufficiently non-obvious, there have been a number of changes as a result of these discussions since the unix epoch.

As a result most applications fall into 2 categories:

• some kind of atomic time counter, so deltas make sense.
• A human readable date time that kept sync with the day/night and seasons and accommodated for time-zones, daylight savings.

And the libraries that do the conversion, tend to be a complex 'best-effort' affair. There are none that I know of that for example do leap-years and time zones but not DST that are universally present in the standard library. The reason being, most of the time the human readable form came from a system time that's regularly updated, has drift problems, is subject to all these changes and complexities etc. Hence everything is a guess, and so you may as well have as good a guess as possible.

For example where I am:

• Wed 6 Feb 12:53:20 2002 (GMT) becomes 1013000000
• 2013000000 becomes Sat 15 Oct 15:40:00 2033 (BST)

Whereas

• Wed 6 Feb 12:53:20 UTC 2002 is also 1013000000s after the unix epoch according to the standards

but

• Sat 15 Oct 14:40:00 UTC 2033 is the time 2013000000s after the unix epoch.

For your test cases though the answers are not so nuanced. I guess they are based on one particular location. I.e.: the tests are of the form:
time_t expected = construct_date(2009, 2, 19, 1, 46, 40);
time_t actual = gigasecond_after(construct_date(1977, 6, 13, 0, 0, 0));
TEST_ASSERT(expected == actual);

But as seem from above: the question of: what time/date comes 1e9 seconds after "6 Feb 12:53:20 2002" doesn't have an answer. (It does have an answer in UTC, which seems to be the answer you are after.)

I don't see a way around this issue if you need to use one of those libraries that "just takes care" of the conversion. You will need your own (gmtime() aside as discussed).

There is a separate point I was getting at by:

Note: if using GMT is not fine, and you need the time_t to be correct for your local time, this can be fixed by offsetting with mk_time of a zeroed out struct tm. I can make this change if someone wants.

That is the question mostly has the same answer in any fixed timezone. The only exception I can see is if the start or end time would have include a leap second differently depending on the timezone.
However, the intermediate time-since-epoch will be wrong, if you assume UTC was used.
You can guess at this correction with mktime(), but it won't change the answer or the need for the extra code. If the mostly isn't a deal breaker an the intermediate is, then this trade-off can be made.

[ryanplusplus]

Thanks @DrJPizzle. I think for now we should go with these changes. Long term we can wait to see if there is a change in the exercise in the common repo or look at simply dropping the exercise.

Can you run indent.sh so that Travis CI passes?

[ryanplusplus]

Thanks!

[kotp]

Interesting discussion. Currently going through some changes in the common repo here.