Thursday, Jun 24, 1:00PM

[karzilla]

Bug counts: resolved 191, assigned 8, needs-review 7

Today's random cleanup project:

Most people want to print out dates in ISO format, but there's no standard way in the code to do this, so we see code that looks like what just went through changelog for bug 1561:

  my ( $sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $isdst ) = gmtime( $u->timecreate );
  my $journalcreated = sprintf( "%04d-%02d-%02d", $year+1900, $mon+1, $mday );

If there's one lesson I want you to take away from my first post in this journal, it is that code duplication makes me twitch. In this case, it's unlikely that we're ever going to need to update that code (barring some major apocalyptic event rewriting our dates to count from a year other than 1900), but it seems like a good idea to make this a function, just so that people have to write less code every time they want to do this.

I will call my new function LJ::mysql_date - because it is a direct analog to LJ::mysql_time, which already exists in ljtimeutil.pl and does the same thing, except including the time instead of just the date string. I think iso_date and iso_time would be better names, but I'm not prepared to rock the boat that much today.

So now the above code can be made much simpler:

  my $journalcreated = LJ::mysql_date( $u->timecreate, 1 );

The '1' indicates we are using gmtime instead of localtime, as mysql_time also does.

Having done that, another simplification presents itself. Every ID being calculated in this code starts with the exact same form "tag:$LJ::DOMAIN,$journalcreated:$u->{userid}" which is calculated from user data and the site constant $LJ::DOMAIN. That makes it a good candidate for a user method, instead of constructing the string from scratch everywhere it is needed. Furthermore, if we ever need to change the format again (as this bug just did), we can update it in one place.

I will call it $u->atomid and define it as a subroutine in LJ/User.pm, which is the large file where most of the user method declarations live. I will add it to Section 26: Syndication-Related Functions, since it is related to generating the user's atom feed. There's only one other function there at the moment!

kaisa did a great job in the original patch of changing the affected lines from the old format to the new format. I'm simply looking one step further, to say "this should have been packaged more neatly in the first place."

My bug for this work is 2738 and you can view the full patch there.

Comments

[alierak]

mysql_date might need a desc / usage comment.

[kareila]

... Really? It's not blindingly obvious?

Hmm.

Okay, fixed.

[alierak]

Thanks. I read the patch before your post, and my immediate response was like "mysql the who what now?". Because, you know, it totally gets a db handle and selects DATE(NOW()) or something...