|
Do 24:00 and 00:00 occur at the same time then, just different notations?
|
|
|
|
|
Yes, different notations for the same timepoint.
|
|
|
|
|
Same timepoint, but different datepoint
modified 19-Nov-18 21:01pm.
|
|
|
|
|
The Army (at least) took 2400 as the end of the day...and never acknowledged 0000 as existing. :p
|
|
|
|
|
Then one second after that would be 240001 or 000001?
|
|
|
|
|
000001
But we just started the new logs at 0001Z. That minute inbetween was just ignored.
Actually, come to think of it, we never used 2400Z, either. The day ended at 2359Z and the next started at 0001Z. :p
|
|
|
|
|
If you are dealing with instantaneity, 12:00:00 and 24:00:00 (or 0:00:00, if you prefer) are neither AM nor PM: the first is Noon and the second is Midnight.
Just think of it as evolution in action.
|
|
|
|
|
But there is a fix for that issue...
string ampm = GetTime("Blahblahblah", strTime);
if (strTime = "24" && ampm = "PM")
{
ampm = "AM";
}
See! It's easy.
|
|
|
|
|
I agree that the original implementation is bad but beware that your fix could introduce some new issues. For example passing "2" will result in a different result than before ("AM" vs ""). This might even be Ok but passing "test" will generate an exception where the old implementation would just have returned an empty string.
Robert
|
|
|
|
|
I am not changing the existing app at the moment .. my job is to add fixes to the app if it fails but it is working OK since last 2 years and used as production app in factory to control robots...One of the developer responsible for its maintenance is leaving so all his projects are coming to me...Just for this app my approach is going to be " Hey Why change it if it is working " ... and there are at least 5 developers worked on it at different times with their own way of coding like the Gem I found...
Zen and the art of software maintenance : rm -rf *
Math is like love : a simple idea but it can get complicated.
|
|
|
|
|
That is quite spectacularly bad.
- Unused parameter
- Misnamed function (it is getting the hour part suffix, not Time)
- Using strings to manage date/time info, and relying on the exact string (i.e. 2 != 02)
- Incorrect coding or non-standard representation (01-24 instead of 00-23)
- Duplicating framework functionality (myDateTime.ToString("hh tt"))
- Repeat code in case blocks instead of a single test for all included conditions
Anyone spot any more?
|
|
|
|
|
I laughed out loud when I saw this one...
Possibly the funniest function I've seen in the hall of shame.
|
|
|
|
|
Some more:
- no test to verify input value range or format (this would be neglectable if using correct types as you've already implied with your point 'Using strings')
- using switch without default case
- using hard coded strings for "AM" and "PM"
- combining a yes/no test with an unrelated display functionality into one function (note that a test for AM and PM implies that a 24h format is being converted to a 12h format, and thus the test needs to be repeated in order to convert the number part!)
|
|
|
|
|
BobJanova wrote: Anyone spot any more?
No need to set a string to return from the function. A simple return "AM"; or return "PM"; would suffice.
I'm not a programmer but I play one at the office
|
|
|
|
|
That's not really so much a horror as a stylistic choice, though one which I would also change as you have suggested there. Some people say that a single point of return is better because you don't have to look through the whole function to find escape points, and though I disagree with that (for one thing your function should be short enough it isn't a problem) I can see what they're getting at.
|
|
|
|
|
I disagree with the single point of return as well. To me it requires some degree of convoluting the code just to reach that point. I tried that approach for a while but I found myself having to add code rather than getting straight to the purpose of it. But then I've always tried to avoid having to take those few extra CPU cycles to get from point A to point B if point A++ gets the job done faster.
I'm not a programmer but I play one at the office
|
|
|
|
|
This is why you should never work past 24.01h.
Giraffes are not real.
|
|
|
|
|
maybe the programmer was paid by the line
|
|
|
|
|
Dude
You got to be making that up!
"To alcohol! The cause of, and solution to, all of life's problems" - Homer Simpson
|
|
|
|
|
The AM/PM suffix only makes sense on a 12-hour system, not a 24-hour clock. If you use an 24-hours based day, you already know whether the time is in the morning or evening. Otherwise, the 8 O'Clock needs the AM/PM suffix to discriminate between 8:00 (8 AM) and 20:00 (8 PM).
It's a ridiculous format, and that mistake is as grave as the function is coded. It'd be best to use the date-format as specified in the users' settings, or to use a predefined format. Making up your own format requires owning a country.
Bastard Programmer from Hell
|
|
|
|
|
Eddy Vluggen wrote: The AM/PM suffix only makes sense on a 12-hour system
true
|
|
|
|
|
Or... Assuming the string has only military time with no date:
string strReturn = DateTime.Parse(DateTime.MinValue.ToString("MM/dd/yyyy ") + strTime).ToString("tt");
If it has date and time:
string strReturn = DateTime.Parse(strTime).ToString("tt");
Kevin Rucker, Application Programmer
QSS Group, Inc.
United States Coast Guard OSC
Kevin.D.Rucker@uscg.mil
"Programming is an art form that fights back." -- Chad Hower
|
|
|
|
|
your solution is different from original.
What if the parameter string is not a number or number any other than one in switch?
The original will return empty string while yours - eather throw or return PM instead.
|
|
|
|
|
I know what you mean mate... the solution I put was just to show that whole function can be redundant. If I have to code it I will make sure of all exception and validations on client and server...
Zen and the art of software maintenance : rm -rf *
Math is like love : a simple idea but it can get complicated.
|
|
|
|
|
You should focus on making it future-proof.
I wanna be a eunuchs developer! Pass me a bread knife!
|
|
|
|