CodeSOD: The Data Class

There has been a glut of date-related code in the inbox lately, so it's always a treat where TRWTF isn't how they fail to handle dates, and instead, something else. For example, imagine you're browsing a PHP codebase and see something like:

$fmtedDate = data::now();

You'd instantly know that something was up, just by seeing a class named data. That's what got Vania's attention. She dug in, and found a few things.

First, clearly, data is a terrible name for a class. It'd be a terrible name if it was a data access layer, but it has a method now, which tells us that it's not just handling data.

But it's not handling data at all. data is more precisely a utility class- the dumping ground for methods that the developer couldn't come up with a way to organize. It contains 58 methods, 38 of which are 100% static methods, 7 of which should have been declared static but weren't, and the remainder are actually interacting with $this. All in all, this class must be incredibly fun".

Let's look at the now implementation:

class data{ // ... public static function now() { return date('Y', time())."-".date('m', time())."-".date('d')." ".date('H').":".date('i').":".date('s'); }}

Finally, we get to your traditional bad date handling code. Instead of just using a date format string to get the desired output, we manually construct the string by invoking date a bunch of times. There are some interesting" choices here- you'll note that the PHP date function accepts a date parameter- so you can format an arbitrary date- and sometimes they pass in the result of calling time() and sometimes they don't. This is mostly not a problem, since date will invoke time itself if you don't hand it one, so that's just unnecessary.

But Vania adds some detail:

Because of the multiple calls to time() this code contains a subtle race condition. If it is called at, say, 2019-12-31 23:59:59.999, the date('Y', time()) part will evaluate to 2019". If the time now ticks over to 2020-01-01 00:00:00.000, the next date() call will return a month value of 01" (and so on for the rest of the expression). The result is a timestamp of 2019-01-01 00:00:00", which is off by a year. A similar issue happens at the end of every month, day, hour, and minute; i.e. every minute there is an opportunity for the result to be off by a minute.

It's easy to fix, of course, you could just: return date('Y-m-d H:i:s');, which does exactly the same thing, but correctly. Unfortunately, Vania has this to add:

Unfortunately there is no budget for making this kind of change to the application. Also, its original authors seem to have been a fan of code reuse" by copy/paste: There are four separate instances of this now() function in the codebase, all containing exactly the same code.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!