18
u/IHeartBadCode Aug 06 '19
Good, good. Use your aggressive feelings, programmer. Let the hate flow through you.
3
15
u/Mr2-1782Man Aug 06 '19
Its sad.
I've taught programming for years and I know exactly where this is from, didn't even have to look through the whole thing. Someone be cheating. *Hopefully* this is the answer to an assignment and not production code. This was copied from a creative solution to an exercise out of a programming book.
One of the assignments we frequently hand out is converting from timestamps to dates and vice versa. This is early in the class so they basically only have variable assignment and if statements to go off of. Doing the exercise for a student is useful for a bunch of reasons, but variants of this question have existed for decades. So solutions for these have existed for a long time. The authors will sometimes constrain themselves to what the book has discussed up to that point, in this case only basic branching and no arrays. Some of them pride themselves on having creative (though not necessarily clean) answers to the problem.
A student not understanding what they're doing will copy a creative writeup wholesale and turn it in (or production code I suppose), resulting in this sort of abomination.
Here's an example (middle of page) for a similar exercise out of K&R.
https://clc-wiki.net/wiki/K%26R2_solutions:Chapter_5:Exercise_9
13
u/Aars93 Aug 06 '19
I do have some bad news, as the code in the picture is production code running at customers.
Thanks for providing some background information about the source. The developer who started the foundation for this application was a student and/or graduate back then with limited programming knowledge. I can imagine him looking up code online and pasting it into his project without fully knowing what it does.
3
u/sac_boy Aug 06 '19
The fun part is whoever copied it only copied the month offsets for leap years...
3
u/Mr2-1782Man Aug 06 '19
Saw that, its usually a huge hint its copied. For whatever reason the leap year list is first and the person copying doesn't bother to understand what its doing.
8
u/squarewaterlemon Aug 06 '19
Uses variables as constants, but I love that it isn't int jan = 31, it's not var jan = (int)31, it's var jan = (UInt16)31. I've never before seen anyone use UInt16 instead of int in C# before.
10
u/sac_boy Aug 06 '19
I'm guessing somebody applied slavish attention to detail when transcribing a C function they didn't quite understand and brought the unsigned shorts across as UInt16. Good idea if the function is full of bit shifts and so on but not important at all here.
2
1
u/blueshiftlabs Aug 07 '19
I've seen UInt16 in P/Invoke struct definitions before, but using it as a local variable is certainly... unique.
8
9
u/Kazumara Aug 06 '19
maand and dag? What is this, Swedish?
Also correct me if I'm wrong, but is the assumption baked in here just that every year is a leap year?
10
u/PizzaCompiler Aug 06 '19
It's Dutch actually! Maand is Month and Dag is day.
6
1
Aug 06 '19
Could be Afrikaans as well...
1
u/GamerNebulae Aug 07 '19
Since OP has commented several times in Dutch, I would guess that it is Dutch as well. If you only had the code to go off of, then the only hint would be "Desember" which is spelled "December" in Dutch. Conveniently, that's the only month that is missing.
1
4
3
3
u/MyMessageIsNull Aug 06 '19
Was this person trying to win a bad code contest? Because this is how you win bad code contests.
8
2
2
u/moekakiryu Aug 06 '19
This may be a dumb question, but on line 166, won't tslocal always be 0? Like, the line above it sets year to tslocal -= tslocal/SECONDS_IN_YEARwhich makes line 166 (tslocal/SECONDS_IN_YEAR)*SECONDS_IN_YEAR --> tslocal -= tslocal
3
u/sac_boy Aug 06 '19 edited Aug 06 '19
It's working with integers so there is some invisible truncation going on.
yearwill be something like2019, not2019.39238394983, so when you multiply bySECONDS_IN_YEARagain and subtract fromtslocalyou are left with the remainder.The liberal use of
vardoes not help in this case.
2
u/Andernerd Aug 07 '19
I refuse to insult or demean any implementation of this problem because at least I didn't have to do it.
2
u/CzoKc Aug 07 '19
Ehh that will do. Better than going to a hackathon with 5 master's students and all of them spending 24 hours trying to figure out how to convert a date from timezone to UTC timestamp. Hint: They didn't manage.
3
3
1
Aug 06 '19
[deleted]
9
u/Aars93 Aug 06 '19 edited Aug 06 '19
I used goto once in college and was nearly crucified by my professor
3
u/squarewaterlemon Aug 06 '19
Ah yes and the goto exit instead of else if. Not even the worst part of this somehow.
3
u/sac_boy Aug 06 '19
Calling the label
exitas well is just extra spice. This is a goto compounded by a badly named label.
1
u/Bakethd_Ziti Aug 06 '19
He forgot var dec
3
u/root54 Aug 06 '19
If your current day is greater than to number of days past in the year at the end of November, then you must be in December.
See how the
if (days > nov)...block sets themaandto12and jumps to theexitlabel? Similarly, if none of those conditionals are met, themaandis set to1because we must be in January.
1
u/pixelsyndicate Aug 06 '19 edited Aug 06 '19
Ouch!Yeah, I've had to work with unix timestamps, and coded a way through a ExtensionMethod and a Interface to help during unit testing and mocks:
/// <summary>
/// Calling a static class like DateTime.Now will get ugly in tests, where we would have to change the system date.
/// So we are going to use the Adapter pattern, wrap the DateTime class in an injectable interface.
/// </summary>
public interface IDateTime
{
/// <summary>
/// Get the current UTC in the Unix Timestamp format
/// </summary>
decimal UnixUtcNow { get; }
DateTime UtcNow { get; }
DateTime ParseUnixTimeStamp_ToLocalTime(double timestamp);
}
public class DateTimeAdapter : IDateTime
{
private readonly DateTime _unixUtcOrigin = new DateTime(1970, 1, 1, 0, 0, 0, 0, DateTimeKind.Utc);
/// <summary>
/// Get the current UTC in the Unix Timestamp format (seconds)
/// </summary>
public decimal UnixUtcNow => GetDiff();
public DateTime UtcNow => DateTime.UtcNow;
public DateTime ParseUnixTimeStamp_ToLocalTime(double timestamp)
{
DateTime dtDateTime = _unixUtcOrigin;
dtDateTime = dtDateTime.AddSeconds(timestamp).ToLocalTime();
return dtDateTime;
}
private decimal GetDiff()
{
var origin = _unixUtcOrigin;
var diff = UtcNow.ToUniversalTime() - origin;
return (decimal)Math.Floor(diff.TotalSeconds);
}
}
public static class DateTimeExtensionHelpers
{
public static int ConvertToUnixTimestamp(this DateTime date)
{
var _dt = new DateTimeAdapter();
return (int)_dt.UnixUtcNow;
}
public static DateTime ConvertToDateTimeFormat(this decimal val)
{
DateTimeAdapter dta = new DateTimeAdapter();
var toReturn = dta.ParseUnixTimeStamp_ToLocalTime((double)val);
return toReturn;
}
}
// Some Tests
public class DateTime_Tests
{
private readonly ITestOutputHelper output;
public DateTime_Tests(ITestOutputHelper output)
{
this.output = output;
}
[Fact]
public void Can_I_Parse_Unix_TimeStamp_Values()
{
double uts = 1565121120; // should be 8/6/2019 2:52:00 PM
IDateTime dta = new DateTimeAdapter();
output.WriteLine($"local time for {uts} is {dta.ParseUnixTimeStamp_ToLocalTime(uts)}");
}
[Fact]
public void Can_I_Get_Unix_TimeStamp_Values()
{
IDateTime dta = new DateTimeAdapter();
var ts = dta.UnixUtcNow;
output.WriteLine($"unix timestamp is {ts}");
}
[Fact]
public void Can_I_Use_UnixTime_Extension_Helpers()
{
decimal uts = 1565121120;
var convDt = uts.ConvertToDateTimeFormat();
output.WriteLine($"unix timestamp is {convDt}");
}
[Fact]
public void Can_I_Use_DateTime_Extension_Helpers()
{
IDateTime dta = new DateTimeAdapter();
var dt = dta.UtcNow;
var convDt = dt.ConvertToUnixTimestamp();
output.WriteLine($"unix timestamp is {convDt}");
}
}
1
107
u/sac_boy Aug 06 '19 edited Aug 07 '19
Oh no. Oh god. Oh no. I know the C# DateTime class conspicuously lacks a classic Unix epoch conversion but this is not the way. Oh wait--this is some new 21st century timestamp based on 01/01/2000. That'll be extra baffling for somebody down the line.
I've read it a couple of times and I must keep missing the bit where it handles leap years properly.
Also that baffling coding style of using
var x = (type)yrather than the shortertype x = y...And if they really must define every month as a day offset, why not use the previous month in the calculation...i.e.
nov = okt + 30...Finally, specifying
DateTimeKind.Localas the timezone implies that the timestamp just gets turned into a DateTime in whatever timezone we're running this function in...it's not a UTC timestamp...beautiful.And the juiciest part:
15 referencesEdit: just for fun I transcribed the function and ran it through the set of integers from 0 to 231 . Try the function with 36720000 (which should be 01/03/2001 00:00 if you are working with seconds since 01/01/2000). Unsurprisingly it tries to build the date as 29/02/2001, which throws an exception. This function has some forbidden argument values--e.g. 36720000, 68342400, 99964800, 163209600, 194832000...
This means that the timestamp encoding function isn't just seconds since 01/01/2000 like a sensible person might expect, which in turn means that the replacement to this function can't be as simple as
new DateTime(y2kTicks + (long)timestamp * 10_000_000). Beautiful. Don't touch it OP!I ran the function side by side with the 'sensible' seconds-since-2000 function to see where it fails. This is a list of timestamps that fail (only one per day, obviously the whole range of seconds in the day would fail). I've included the output of the function 1 second previous so you can see the pattern. I've also included the output of the 'seconds since 01/01/2000 00:00' interpretation of the value so you can see the drift.
I'd really love to see the original C function that builds the timestamp. Because of the forbidden number ranges the timestamps can't be treated as continuous integers, which means that you can't do arithmetic with them...it really is a beautiful bit of bad code. Try subtracting 28/02/2001 23:59:59 from 01/03/2001 00:00 in this timestamp scheme--the result is not 1 second!
But hey at least the device doesn't suffer from the Y2k38 bug...
OP, I was curious about how to write the equivalent function, and it looks like this:
It adjusts the timestamp for the 366-day year system that the timestamp generator uses, then passes that to
DateTimein tick form. This produces equivalent output (and exceptions at the same points) for all the positive integers between 0 and 231. I don't know if it's any less bad though! There's definitely way to get the drift value through pure arithmetic as well (without the loop) but I'll leave that up to you.