5 ms·
God, I seriously hope string-switching doesn't catch on. In a prev job, I saw this block of code - ------- public int getDaysInMonth( String month ) { if(
by dxbydt 15y ago
God, I seriously hope string-switching doesn't catch on.
In a prev job, I saw this block of code -
-------
public int getDaysInMonth( String month ) {
if( month.equalsIgnoreCase("january")) return 31;
else if( month.equalsIgnoreCase("february")) return 28;
else if( month.equalsIgnoreCase("march")) return 31;
else if( month.equalsIgnoreCase("april")) return 30;
...
}
-------
My eyes bled. I said folks, just map the month to an int M in range [1..12] and simply write 1 line of code
return 30+((M+Math.floor(M/8))%2;
( except for 2, which is 28 or 29 depending on a leap year)
I was overruled. "We are programmers, not mathematicians" was the response!!!
Now they'll happily use this string-switch to do
---------
switch( month.toLowerCase() ) {
case "january" return 31;
case "february" return 28;
case "march" return 31;
case "april" return 30;
...
default : return -1;
}
-------
which is an even nastier abomination :(
- chubot 15y agoAre you serious? An "abomination"? The switch statement is fine, and using an array [31, 30, 31 ...] would be fine as well. Your 1 line isn't really 1 line because you have to convert the month to an integer anyway, and then the array lookup is even shorter than your solution.
- VMG 15y agoYour one-liner is more difficult to understand. The string switch is very explicit and understandable.
- bethling 15y agoI have to admit - I never even thought that there would be an easier math-formula way to calculate it. Though I would have written the switch: case "september": case "april": case "june": case "november": return 30; It's sad, but still need to go through the poem in my head to remember most of the time.
- oleeang 15y agoUse the knuckles trick: http://lifehacker.com/232828/macgyver-tip-use-your-knuckles-to-remember-each-months-days http://lifehacker.com/232828/macgyver-tip-use-your-knuckles-...
- Bootvis 15y agoTo be honest your one liner takes me a while to parse and you forgot to close a parenthesis. OTOH It is really hard to read or write that switch incorrectly.
- kragen 15y agoI have to say, I think it's easier to verify the correctness of [31, isLeapYear(year) ? 29 : 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31][M-1] than your formula. Even a dictionary wouldn't be bad. And if it were wrong, it would be easier to fix it.
- fleitz 15y agoCorrectness should be verified by tests not 'looking at the code'. The aforementioned code can never be verified as correct because February requires a year to be present to know the number of days. Doesn't java have a massive calendar library to address exactly these sorts of human questions? I mean sure you'd still be looking at code like: public int getDaysInMonth(DateTime date) throws CalendarFactoryFactorException, DateOutOfRangeException, ArrayOutOfBoundsException { CalendarFactoryFactory factoryMaker = new CalendarFactoryFactory("Western"); GregorianCalendarFactory gregFactory = factoryMaker.getGregorianFactory(); Calendar c = gregFactory.createCalendar(new DateTime()); return c.getDaysInMonth(); }
- kragen 15y ago> Correctness should be verified by tests not 'looking at the code'. Tests can prove the presence of bugs, but they are (in general) hopelessly inadequate to prove their absence; and once a test finds a bug, you still have to be able to understand what the code is doing in order to fix the bug. In practice, code inspections and tests find quite distinct sets of bugs with some overlap, so it's valuable to do both of them. That's why all decent programming processes require both. (This is probably one of the few cases where you could in fact verify that the code is correct by exhaustive testing — but only by comparing it against a known-correct implementation.)
- hkolek 15y agoYes, something like that. But better use joda time because the standard API sucks. joda: new DateTime().monthOfYear().getMaximumValue()
- narayanbabu 15y agoOne code review session is equivalent to 100 debugging sessions, so your statement that [Correctness should be verified by tests not 'looking at the code'] is very misleading.
- jganetsk 15y agoWhy did you need string-switching? You could have created a Map<String, Integer> lookup table.
- FaceKicker 15y agoUsing a contrived formula that's difficult to understand or quickly check for correctness to solve a problem whose answer doesn't come from anything mathematical (i.e., the number of days in each month were arrived at arbitrarily, not via any formula) and just happens to work is NOT good style. At best it's cute and irrelevant (and doesn't even work for all of the twelve cases...). Wow. Edit: also, if you're mapping the month string to an integer anyway, I don't see how you're going to do that without doing basically the same thing as the switches you posted anyway.
- reinhardt 15y agoHope I never have to maintain pseudo-mathematical code like the one you were (thankfully) asked to revert.
- pasodoble 15y agoThis one is easier to understand imho: 30+(1&(5546>>M)) (also except for M=2)
- dxbydt 15y agoWell done! Could you elaborate on how you came up with 5546 ? Perhaps there's a simpler technique than the one I describe below. btw, this was the technique that I learnt in school - 1. You have 12 months. Ignore February. Then you have 11. 2. The remaining 11 map to either 30 days or 31 days . ie. 30+0, or 30+1. 3. So the object is to write a many-to-one function that maps some members of the set to 0, rest to 1. Add 30 and you get the right answer ie. number of days in that month. 4. So you want to map {1,3,5,7,8,10,12} to 1, and {4,6,9,11} to 0. 5. Write down a 13 bit number. If you rightshift x times and AND it with unity, you get 1. 6. So then, x belongs to {1,3,5,7,8,10,12}. 7. That means the bit in each of those locations must be on. The rest of the bits must be off. That gives you the number 1010110101010, which is simply 5546 in decimal. 8. So that's ((5546>>M)&1)+30, except for M==2.