| « Prev | Page 1 | Page 2 | Next » |
|
Well, given that the function is named "weekday", and that the last calculation is a mod-by-7, I'd assume that the function returns the weekday of a given date, not a UNIX timestamp.
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 10:22
•
by
JamesKilton
|
|
Took me 5 seconds to find (on php.net) mcal_day_of_week($year, $month, $day) http://us2.php.net/manual/en/function.mcal-day-of-week.php
/sigh |
|
This is one of the reasons why IT people in the western world (the free world according to a certain American president) will still have a job after the deployment of "offshore developer centers" (ODC) in every major IT company. If it is any concelation to David, a certain ODC in India could have done the same. I have seen it myself. regs, Gnoople - Employed system developer in Scandinavia. |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 10:35
•
by
diaphanein
|
Yes, offshoring is actually a clever ploy to raise salaries of onshore programmers whilst eliminating those lowly html "programming" jobs. |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 10:40
•
by
Zylon
|
Eh, that line has been around for decades. And should I even be surprised that there's a Wikipedia article on it? |
|
Excuse me while I vomit.
Code like this from "offshore" developers is in line with my experience. We always get told that these "offshore" developers have such a great education. But they know shit. In this case they have apparently never heard of Zeller's congruence algorithm. Yeah, just memorizing the name and a few control constructs of a language doesn't make you a good programmer. |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 10:49
•
by
Gnoople
|
It doesn't make it less true. The fearless leader of US is a true liberator of (oil producing) countries in the middle east. But, enough politics for now, I just couldn't resist. :-) I still think ODCs suck, even if they contribute to cooler programming tasks and higher wages.. |
|
|
Actually, this code is supposed to kill you |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 11:03
•
by
SeeJay
|
Dunno about leader, but I certainly prefer Rocking in the Free World instead. XD |
|
I think I see the WTF: the function will treat 1900, 2100 and friends as intercalary years.
|
|
I've seen worse date code written by inshore staff, but I do like how unintentionally cryptic this one is. Might just have to copy-paste for some job security.....
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 11:13
•
by
KattMan
|
|
Actually, code like this is the accepted norm. Offshoring is just filling the need of the majority of companies. They don't want it done right, just right now. |
|
It's the magic numbers that send shards of ice through my spleen. And don't start me on ODC's - I'd rather dip my testicles in a chip fryer than have to refactor some of the "code" we've been sold.
|
|
Given the huge function libraries available for most modern languages, it's not surprising to find people who don't know everything that's available for the language they use. I personally don't hold that against anyone. But not knowing to type 'google' before launching into writing something like that is unforgivable.
|
another example of the "you touch it, you own it" law |
Some folks tend to take the concept of magic numbers a bit too far. Yes, we could make the number of days in a week a constant, but really, do we ever really need that much generality? On the other hand... |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 11:30
•
by
cconroy
|
Hmm, you must have the Code Snippet of the Day confused with the
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 11:34
•
by
eight days a week
|
Making the number of days in a week a constant could add some clarity as a programmer would immediately know what the 7 meant, but that's a pretty minor point. I'd like to know what the meaning of this magic number is: What is -473?
|
|
I think the function 'weekday', whose implementation ends in '% 7', and that compensates
for leap years, just does that: it computes the weekday. It looks copy-pasted from a cookbook or other source. That the submitter thinks it relates to Unix time_t's, and plans to rewrite the module based on that idea, more than justifies the outsourcing decision than anything else.
|
|
Ah yes, yet another instance of the wheel being reinvented for the 59,000th time. Honestly, no programmer using any modern language should have to write any sort of time calculation code. And when someone does do it, it invariably is wrong in some way due to the many idiosyncracies of getting date/time calculation correct. I don't program in PHP too much myself, but I'm pretty darn sure there are like 4000 different date/time manipulating functions alone for the language. Use them!
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 11:39
•
by
John Cowan
|
|
Not the point. How can you be sure, just looking at that code, that it's correct? Similar routines were discussed and debugged in the Elements of Programming Style, a wonderful little book that took all its examples of hideous code from other textbooks on programming.
|
$leap_years=(intval($corrected_year/4)); ... (intval($leap_years % 25)<0) ? 1 : 0) assuming that $year is always >= 1 (there's no year 0), $corrected_year should be >= 0; so $leap_years >= 0; so when would $leap_years % 25 be < 0? |
|
You might want to blame offshore developers, but I have seen code like this far before the current offshore trend. When I was looking for a C date library, I remember seeing many hideous C, Excel, and Foxpro date algorithms in the same vein as this WTF. I almost puked when the authors called the algorithm the "recommended" method for date calculation. I should really look it up again for posterity -- it would be more fun than the detailed design specification I currently writing. I ended up using a very nice C library, sdncal20, as opposed to abominations such as this WTF.
|
|
Another thing that bugs me is that they're mixing 0-based and 1-based indexing. They're requiring the month to be passed in as a number between 1 and 12, but returning the day of the week as a number between 0 and 6. (1 to 12, 1 to 7) I could understand (very BASIC.) (0 to 11, 0 to 6) I could understand (very C.) But this is confusing.
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 11:54
•
by
Unomi
|
|
It is not like 'Free world' where all people live which are not in prison or have their own choice to go where they want. If you read it correctly, Leader of the Free World means there is a world for grabbing and somebody has taken the first steps to do so. Under the New World Order, "Liberty" and "Freedom" are very different things. They even mean things we never will understand under the powers that be. - Unomi -
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 11:59
•
by
un.sined
|
I didn't know about it either... Of course, now I do. |
mmm... they actually do decrement corrected_year right before saving it to leap_year.. so it very well could be zero. If year = 1 and month < 3 was passed. naturally. |
|
After little contemplation and a few moments testing I KNOW that this code returns a number of the day or the week for a given date, month time. The return code is mod 7 and testing confirms that this code assumes Sunday = 0. So one WTF perhaps is that David may now be rewriting the function to do something completely different.
This was copied and pasted from php.net as another poster noticed. Much cleaner implementation below, except this should handle all the leap calculations for you by letting getdate do most of the work, very old versions of php may be off a bit. function weekday($year,$month,$day) { // Return integer value of day of week, where Sunday=0 $date = getdate(mktime(0,0,0,$month,$day,$year))); $day_of_week_value = $date['wday']; return $day_of_week_value; } |
|
Whats wrong with:
function weekday($year, $month, $day) { return date("N", mktime(0,0,0,$month, $day, $year)); }
CAPTCHA: poprocks? hoot! |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 12:07
•
by
Corey
|
|
Whats wrong with:
function weekday($year, $month, $day) { return date("N", mktime(0,0,0,$month, $day, $year)) % 7; }
edit: forgot the %7... doh...
CAPTCHA: poprocks? hoot! CAPTCHA2: zork... |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 12:13
•
by
Me again...
|
|
Excuse the inline function. I didn't realize WTF mangled plaintext. I'll try again with code tags..
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 12:16
•
by
Russ Ryba
|
|
Good Job Corey, you will the Golf game. :o)
|
|
Everything else looks OK-ish. // all arithmetic is integer arithmetic - some terms reordered |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 12:20
•
by
Abstract Factory
|
|
Actually, read the caveats -- MCAL is not part of core PHP distro, and it's not even available on Windows, so it's highly possible that your code wouldn't work. Personally, I'd go with idate('w', mktime(0, 0, 0, $month, $day, $year)); |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 12:22
•
by
Guything McThingGuy
|
|
Ok, I figured out the what "25" is for. "$leap_years" is the number of leap years from 1..$corrected_year. Thus, "$leap_years/25" = # centuries. Which means, this is trying to do (in words): -473 (no clue) + # days since 1970 + #leap days - (# centuries // this whole thing is supposed to handle the special case leap-years + 0 // # leapyears % 25 is never < 0 + # 400year years + # days in current year + specified day of month converted to 0..30 ) % 7 ) + 7) % 7 // this seems to be the long way of doing x%7 to get the day of week 0..6 BTW, I am a humble java-guy, so this would be done simply as: calendar.get(Calendar.DAY_OF_WEEK) |
Looks more like it does idate("w", mktime(0, 0, 0, $month, $day, $year)) to me, since it's giving the day of the week back. date() would work as well although it would return a string instead of an int, which would not be an issue since PHP would just convert when you used it. Still, best to specify for sure. This code was likely copied verbatim from some online source anyway. The switch on the month is a bit strange since a 2d array would be simpler, IMO. And the math is unusually obtuse at best, but optimized math is usually obtuse. Although this is probably not optimized anyway. Still, figuring out the day of the week is not a minor math trick, so you wouldn't expect simplicity. You would expect a PHP programmer to at least know about the "date" and "mktime" functions though. |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 12:58
•
by
sillverpie
|
That last bit makes sense if your language's % operator would return a negative value for (negative % positive); it forces the result to be non-negative anyway.
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 13:22
•
by
Derrick Pallas
|
There is one more major difference between mktime and this code, typified by that switch statement. |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 13:32
•
by
GrandmasterB
|
Well, if it was actually working he might as well leave it as-is until he has some free time (or an available intern)
|
|
from 'China' ; maybe (probably) they weren't allowed to get to google?
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 15:03
•
by
Harrow
|
Well, I disagree. I would much rather dip AP's goolies in hot fat than clean up ODC. -Harrow. |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 15:03
•
by
darin
|
Am I missing something here. The code does what it's supposed to. If you look inside those mktime routines, you'll see code very similar to this. I've got nearly the same thing in some embedded code (since I can't just steal GPL code that implements mktime). I've got more comments though because it is cryptic to just plug in a bunch of numbers wihtout referring to the original equation and algorithm. The leap year stuff is mandatory. Maybe the developers did not have access to the equivalent functions, or added this for portability Mktime isn't in my Perl reference. I don't have the "mcal" stuff either. The "wrong" parts of it is that it should have factored out the "Julian Date" stuff into a separate function, and then used that to determine day of week (ie, just pull off the "% 7" at the end). Maybe they could have done research and found if there was a builtin library function to do this. But given that this probably took 5 minutes of time to develop after searching the web, it was probably shorter than trying to integrate a third party library and making sure all customers have that library. |
|
No discussion of date computations would be complete without a reference to the root citation for this subdiscipline.
|
Re: [CodeSOD] Making Time for UNIX
2007-01-05 15:23
•
by
darin
|
Wait, I have to do that! No generic C Runtime Library is able to know how to obtain your particular machine's current time and convert that to a standard "time_t". And then there's the cross conversion between different time standards that tend to occur underneath the hood (most file systems don't use "time_t"). And of course the libraries themselves have to be written by someone; and they're often written in modern languages. |
I'd just like to say: great analysis. This is the way the code should have been commented in the first place. |
I hate that: "You find it, so you own it". That's why I'll never stop to help when I see a broken down car full of helpless little kittens, burning to a crisp on the side of the road. -- :) Why can't the CAPTCHA also be displayed as text? That would be so much easier. Look, I'll let you in on a little secret. I have my images turned off and I am not a robot so please let me in. :)
|
|
http://en.wikipedia.org/wiki/Zeller%27s_congruence
CAPTCH genius ... |
Re: [CodeSOD] Making Time for UNIX
2007-01-05 16:45
•
by
Ownage Personified
|
The last +7 %7 isn't a wtf - it's just taking into account dates before 1/1/1970 for which the inner expression is negative (negative number % 7 is zero or negative). + ($leap_years % 25 < 0 ? 1 : 0) isn't a wtf either, it is there because one leap year is otherwise lost in the leap year calculation for pre-1970 dates as -2 / 4 = 0. The real wtf is that this apparently only works correctly for dates from year 1801 onwards. |
| « Prev | Page 1 | Page 2 | Next » |