5 ms·
This article is very important. I encourage everyone to read it, as it raises a lot of good points. In my opinion, the problem it describes come from the vague
by d--b 11y ago
This article is very important. I encourage everyone to read it, as it raises a lot of good points.
In my opinion, the problem it describes come from the vagueness of the concept of elegance, and how counterintuitive it is. It doesn't even have to imply modules or anything like that. In the simplest form it boils down to:
Would you rather write:
if (a) {
doSomething();
if (b) {
doSomethingElse();
}
}
or
if (a && b) {
doSomething();
doSomethingElse();
}
else if (a && !b) {
doSomething();
}
Of course, there is no true answer to that question, and it always depends on the context. But many programmers will never ever consider option 2. And that is for two good reasons: 1, you make one more test, and 2 you duplicate the function call do doSomething(), so your program is larger. So mathematically speaking option 1 is more elegant, it's shorter, it's faster, it's lighter, what's not to like?
Well, multiply the ifs and the elses, and you will soon find out that option 2 is much more readable and changeable, which is the more elegant solution to anyone who's an engineer rather than a mathematician.
The tension it describes is that in every programmer is a mathematician and an engineer endlessly conflicting. This should be a good guidance for which style to use. Is this code a math code, or is it engineer's code. When you can answer that question, you can decide which style to write your code in.
- pantulis 11y ago"Changeable" is the key here. We coders are trained for abstracting things, but real world requirements sometimes change and your perfectly abstract module needs some non-generic functionality. Of course, there are software engineering techniques to solve these problems, but hey you didn't foresee these changes and now need to bill your client for these adjustments. So the trick is achieving a good balance between redundancy and dependencies. This is probably what seniority means in software development.
- d--b 11y agoCouldn't agree more
- zimpenfish 11y agoOption 2 is less changeable if you now also have to consider e.g. `a && c`. I'd argue it was less readable too (two `doSomething()` to consider vs one is bad) but this is largely personal prejudice - I'd always spell things out "noddy fashion" like Option 1 because I am a simple brained developer.
- d--b 11y agoOf course it depends on the case. But the point is that if you chose Option 1, and you have to change doSomething() in to doYetSomethingElse() in the case where a && !b, then you _will_ have to change the structure to Option 2. So in many cases, it's better to go to Option 2 directly.
- agumonkey 11y agoOne should write a boolean refactorer to be able to test exhaustiveness and different nesting patterns.
- MrPatan 11y agoI completely agree. It's _vital_ to know what a, b, doSomething and doSomethingElse actually stand for, and what is the context. This is not a technical problem, it's a people problem (like almost all problems). Take this, for example: if (userLoggedIn) { setCookie(); if (itsANiceDay) { tellUSerToGoOutAndPlay(); } } And this: if (itsSunny && itsCold) { putOnSunglasses(); putOnCoat(); } else if (itsSunny && !itsCold) { putOnSunglasses(); } They both kind of make sense! Now let's see the other way around: if (userLoggedIn && itsANiceDay) { setCookie(); tellUSerToGoOutAndPlay(); } else if (userLoggedIn && !itsANiceDay) { setCookie(); } This one is just silly. We are mixing logic about two completely different things. Nobody would do that (many people would do that, I know. Too many) if (itsSunny) { putOnSunglasses(); if (itsCold) { putOnCoat(); } } This one seems ok. Until you wonder what happens if it rains and/or it's windy. Then you'll refactor it to a "switch/case" for clarity. And it's all about the domain specific content! You cannot reason only about the syntax tree in isolation and come to a meaningful conclusion!
- jessaustin 11y agoThe problem with these examples is that putOnCoat() should not depend in any way on itsSunny, so there should just be two separate if blocks. We should be discussing a scenario in which one condition is a subset of the other.
- JustSomeNobody 11y agoWhy not? I've been on Rainier when it's cold and sunny.
- Jtsummers 11y agoBut it's an independent test. You'll put on your coat when it's cold, whether it's sunny or not.
- denis1 11y agoI you didn't understand what the GP said. You can certainly have a day when it is both sunny and cold, but the checks would look like this: if(isSunny){ putOnGlasses(); } if(isCold) { putOnCoat(); }
- Dylan16807 11y ago>Well, multiply the ifs and the elses, and you will soon find out that option 2 is much more readable and changeable, which is the more elegant solution to anyone who's an engineer rather than a mathematician. Disagree. In the first case I know only one block (and its parents) can execute. When all the ifs are isolated, it takes much more reading to know that only one will trigger, and that's after I make sure 'doSomething' won't change a or b. The real way to write that is probably a switch statement.
- userbinator 11y agoWell, multiply the ifs and the elses I would consider that doing so makes it a different problem, and so what becomes the better solution changes. If you need very many different combinations, you should probably use a table-based approach instead (produce an integer from each of the booleans, index into a table of function pointers or similar; you can often reduce its size if you put "don't cares" into the more significant bits.) I think the second option is more difficult to understand since you now have to consider together all 4 different combinations of a and b to determine what code gets executed for each one. For the first option, it is immediately obvious that none of those functions execute if a is false, whereas in the second you have to inspect each if-condition.
- Retric 11y agoThey both share a dependency due to nesting or the else statement. A third option may be more maintainable depending on context. if (a) { doSomething(); } if ((a && b)||c) { doSomethingElse(); } Granted, this is a case where refactoring is fairly trivial. PS: The real question is does doSomethingElse depend on doSomething.
- deleted 11y ago[deleted]
- phn 11y agoI don't like repeating function calls, if logic calls for complex conditions (which I don't think the example does) I find it better to express them in new booleans before the if, keeping the logic contained there, and call them on a: bool shouldDoSomethingElse = a && b; if(a) doSomething(); if(shouldDoSomethingElse) doSomethingElse(); kind of thing.
- seanwilson 11y agoI like this technique for avoiding comments in code as the variable name describes what the condition is checking but I don't see it used often for some reason.
- RussianCow 11y agoI like it too, but I assume it's probably not used very often just because it forces you to write more boilerplate code, which some may see as "verbose".
- seanwilson 11y agoOh, I was only commenting on the use of the variable to store and label a condition. For example, I prefer: var insideEventTime = time > start && time < end; if (insideEventTime) ... To this: // When inside event time if (time > start && time < end) ... I find comments frequently become inaccurate compared to function and variable names.
- RussianCow 11y agoI understand what you're saying, but I can see certain devs having a problem with introducing extra variables only to use them in a single conditional. Not that I agree with that, but that's the only reason I can come up with.
- seanwilson 11y agoWhat's wrong with introducing an extra variable compared to introducing a comment?
- overgard 11y agoI generally find that if statements with multiple tests are a frequent source of bugs; because it's easy for someone to add another term to one test without realizing they need to update the other. (Especially if the function is long. Yeah you should write short functions, but long functions happen.) Your second example is much harder to reason about if someone comes in and adds: if ( (a && b) || c) So then what happens with the second clause? (Or what even should happen with the second clause?) On a meta note, any time I see multiple tests in an if statement, I try to pull it into local booleans that describe the intent instead. At the very least then if you add more conditions to the if statement, you have to update the names of the boolean for it to make sense, and so the code stays implicitly commented.
- iamcurious 11y agoNote that your examples are equivalent only if neither doSomething nor a modify b.