Gem... As time goes by..
-
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
-
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
In any case I would also look at all places where the function is called. There this unused parameter must be passed. It's better when you find out what this 'ShiftCode' may have been intended for.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"And I smiled and was happy
And it came worse. -
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
-
It's also wrong : "24" is "AM" :) Cheers
If you can read this, you don't have Papyrus installed
-
And that again may also be wrong. Are the hours not enumerated 0 - 23, totally VB unfriendly?
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"And I smiled and was happy
And it came worse. -
And that again may also be wrong. Are the hours not enumerated 0 - 23, totally VB unfriendly?
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"And I smiled and was happy
And it came worse.Yes, you and I (and probalby most people) would calculate that way. In the past I have once or twice seen the time expressed as 24:xx. Anyhoo, if you inspect the code you will see 11 AM's and 13 PM's. Cheers
If you can read this, you don't have Papyrus installed
-
Yes, you and I (and probalby most people) would calculate that way. In the past I have once or twice seen the time expressed as 24:xx. Anyhoo, if you inspect the code you will see 11 AM's and 13 PM's. Cheers
If you can read this, you don't have Papyrus installed
Time and dates can be hell, even if you have a decent framework at your disposal. If you do such things on the fly: Good luck.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"And I smiled and was happy
And it came worse. -
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
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 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.
-
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
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?
-
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.
-
It's also wrong : "24" is "AM" :) Cheers
If you can read this, you don't have Papyrus installed
You never get "24" it's "00"
my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"
-
You never get "24" it's "00"
my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"
That depends entirely on the input, we're not dealing with a real time format here. As said by others here, thse method is wrong in many respects. My comment denoted that there are 11 "AM" entries and 13 "PM" entries. So if "24" is input, the output should logically be "AM", not "PM". Cheers
If you can read this, you don't have Papyrus installed
-
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
-
It's also wrong : "24" is "AM" :) Cheers
If you can read this, you don't have Papyrus installed
ISO 8601 allows 24:00 as the end of the day, so it would be PM; AM starts at 00:00 (not that ISO 8601 recognizes AM/PM of course).
-
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
maybe the programmer was paid by the line
-
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?
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!)
-
ISO 8601 allows 24:00 as the end of the day, so it would be PM; AM starts at 00:00 (not that ISO 8601 recognizes AM/PM of course).
Do 24:00 and 00:00 occur at the same time then, just different notations?
-
Do 24:00 and 00:00 occur at the same time then, just different notations?
Yes, different notations for the same timepoint.
-
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";switch (strTime) { case "01": strReturn = "AM"; break; case "02": strReturn = "AM"; break; case "03": strReturn = "AM"; break; case "04": strReturn = "AM"; break; case "05": strReturn = "AM"; break; case "06": strReturn = "AM"; break; case "07": strReturn = "AM"; break; case "08": strReturn = "AM"; break; case "09": strReturn = "AM"; break; case "10": strReturn = "AM"; break; case "11": strReturn = "AM"; break; case "12": strReturn = "PM"; break; case "13": strReturn = "PM"; break; case "14": strReturn = "PM"; break; case "15": strReturn = "PM"; break; case "16": strReturn = "PM"; break; case "17": strReturn = "PM"; break; case "18": strReturn = "PM"; break; case "19": strReturn = "PM
Dude :doh: You got to be making that up!
"To alcohol! The cause of, and solution to, all of life's problems" - Homer Simpson