16 ms·

Special Cases Are a Code Smell

198 points by fagnerbrack 8y ago

jimmaswell 8y ago
Certainly subjective. One could in many instances easily find it clearer making the special cases explicit instead of messing with the data beforehand in order to force it to fit into a more general algorithm, these instances included.
PaulHoule 8y ago
The structure of common sense knowledge is "A unless B unless C unless D..." Thus the ultimate way to organize a program is to have a clear separation between the specific and the general.
clusmore 8y ago
I'm reminded of Linus Torvald's interview at Ted [1] where he talks about programming "taste", and compares the following two code snippets: remove_list_entry(entry) { prev = NULL; walk = head; while (walk != entry) { prev = walk; walk = walk->next; } if (!prev) head = entry->next; else prev->next = entry->next; } remove_list_entry(entry) { indirect = &head; while ((*indirect) != entry) indirect = &(*indirect)->next; *indirect = entry->next; } [1]: https://www.youtube.com/watch?v=o8NPllzkFhE https://www.youtube.com/watch?v=o8NPllzkFhE
- gumby 8y ago
  That first case is hideous (hope Linus agreed with me!) but does remind me of my favorite computer scientist joke: === The physicist is showing his friend the programmer a thermos. "You see, you can put a hot drink inside, and then take it with you. It doesn't matter how cold it gets outside; when you pour the drink out it's still hot!" The programmer is quite impressed. The physicist continues, "Or you can put a cold drink inside, and then take it with you. It doesn't matter how hot it gets outside; when you pour the drink out it's still cold!" Now the programmer is really dumbfounded. He asks, "But how does it know?" === Unfortunately though I like the joke, I still see waaay too much code written by the programmer in the joke.
  - foota 8y ago
    Is the joke that the programmer interprets it as the drink waiting to pour out? Or?
    - gumby 8y ago
      A non-programmer will just figure the programmer is a bozo (you could tell this joke with a physicist and a geologist, or any combination of <local-in-group-0> and <local-out-group-0>): that's simply how thermoses work. But a thermos with a switch on the top to say "insert hot" or "insert cold" would be absurd. A thermos is simply a container _with no concept of temperature_, just isolation: it just maintains the state blindly; whether that state is hot or cold is managed by the "caller" who inserts and extracts the contents, inspecting them itself if it wants to know what had been put into it. Yet I still see plenty of code that does have a variable to indicate what's inside, perhaps an enum {hot, cold} or worse a bool. Yuck -- you should simply store the thing and if you need to know its state you look at it. That variable is something to get out of sync; something you need to be careful to set and maintain, something at threat to a race condition or ill-timed interrupt. Bleagh. If your first language was FORTRAN, OK. Otherwise there is no excuse. Sadly I see shit like that in pedagogical examples too, which is malpractice.
      - warent 8y ago
        "I still see plenty of code that does have a variable to indicate what's inside, perhaps an enum {hot, cold} or worse a bool. Yuck -- you should simply store the thing and if you need to know its state you look at it." Now I'm confused... How are those two things different? The variable indicating what's inside is going to be coming from the actual state itself, no? It sounds like perhaps you're talking about some case where a developer has somehow copied a state and referred to the copy somewhere else. Or are you talking about a computed variable of some part of the state being a code smell?
        AstralStorm 8y ago
        He's talking about explicit state, which can be a code smell when you can have equality efficient (or more) stateless solution.
- westoncb 8y ago
  This is an especially interesting comparison because while it does clearly exhibit the sort of elegance only achievable through the elimination of special cases, it's also a good demonstration of the tradeoffs one sometimes has to make for this purpose. The second is significantly 'cleaner,' but with pretty bad readability [0]. The first version has excellent readability. In practice, the way I'd evaluate the two is by looking at how much other code depends on (or at some point may depend on) the piece under consideration. If nothing else or only very little depends on the code, I'll opt for readability; if it's likely to be somewhat foundational and other stuff is going to grow on it, I'll opt for the more abstract, cleaner version, and document thoroughly. [0] I'm guessing the readability isn't nearly as bad if the first example is using a common C idiom; I don't use C enough to know. My main criterion for judging readability is: how much does the structure of the code match the conception of the algorithm you're starting with. The conception here has to do with navigating links; the pointer stuff is incidental complexity.
  - im3w1l 8y ago
    To me the main readability problem of the first example is that it does the head check as the last step rather then first step. Meaning we have to consider "wait, when is this null again". The main problem with the last one is that it's not immediately obvious what the "indirect" represents. When it clicks that it is "the pointer to potentially update", it's pretty easy to understand.
    - ummonk 8y ago
      The 2nd one would be significantly more readable with explicit typing (to make it clear that you're dealing with a pointer to pointer). Though I think putting the head check at the beginning like you said will make the first example very reasonable (and relatively compact too). remove_list_entry(entry) { if (head == entry) { head = entry->next; } else { prev = head; while (prev->next != entry) { prev = prev->next; } prev->next = entry->next; } }
- userbinator 8y ago
  There's a third alternative, which I've heard of as "virtual head", that removes the extra indirections in the loop and removal: while(p->next != entry) p = p->next; p->next = entry->next; I've deliberately not shown the initialisation of p, because it's a bit tricky in C (but trivial in Asm); p is not initialised to the head, nor the address of the head, but to a "virtual" node centered around the head, such that p->next will initially access the head. If the next field is at the beginning of the structure, p does point to the head; else p points to a location before the head. It would be something like (char*)&head - offsetof(Node, next);
  - DerekL 8y ago
    You can get the same effect by using a “dummy” node. The dummy node has a next pointer, but no data, and the dummy node always exists at the front of the list. The list structure contains the dummy, instead of using a pointer to the front of the list. struct Node { struct Node* next; }; struct DataNode { struct Node* next; struct Value value; }; struct List { struct Node dummy; // You can also add length, etc. }; // entry must be in list void remove_list_entry(struct List* list, struct Node* entry) { Node *prev = &list->dummy; while (prev->next != entry) { prev = prev->next; assert(prev != NULL); } prev->next = entry->next; free(entry); }
  - rovolo 8y ago
    For those curious how to create the "virtual head" in C: struct entry { int value; entry * next; }; entry * head = NULL; void remove_entry(entry *target) { long offset = (long)&((entry*)0)->next; entry * curr = (entry*)((long)&head - offset); while (curr->next != target) curr = curr->next; curr->next = target->next; } But I'm not sure how portable that is. It is portable if the next pointer is the first member of the struct though. From 6.7.2.1.13 in the C99 standard: >A pointer to a structure object, suitably converted, points to its initial member struct entry { entry * next; int value; }; ... // virtual entry where &(curr->next) = &head entry * curr = (entry*)&head; Note that it's exactly the same logic as the grandparent since *curr = curr->next indirect = curr *indirect = *curr *indirect = curr->next indirect = &(curr->next) But, I think that the virtual head entry is an easier mental model. C99 standard: http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf
gumby 8y ago
These suggested fixes look worse to me. In the first one, the "wrong" function was quite clear (special case 0, special case 1, general case) while the second one allocated memory. I'd call that a code smell!
- eridius 8y ago
  Agreed. A better solution here is to have a generic accessor on the Array that can return a default value for out-of-bounds accesses. There almost is one already, the two-argument version of fetch(), except that interprets negative indices as counting from the end. But if we create a variant that doesn't treat negative indices specially then the code becomes very simple result = input.map.with_index do |_, i| input.better_fetch(i-1, 0) + input.better_fetch(i+1, 0) end.to_a
  - chrismorgan 8y ago
    Yeah, in Rust it’d fall very neatly out of the type system, as `input.get(i - 1).unwrap_or(0) + input.get(i + 1).unwrap_or(0)`.
    - eridius 8y ago
      Funnily enough, that Rust code is actually buggy, because i is a usize and if i == 0 then i-1 will panic due to underflow. You'd have to write input.get(i.wrapping_sub(1)).unwrap_or(0) + input.get(i+1).unwrap_or(0) This also technically won't work right if the slice has usize::MAX elements, but that's rather unlikely. This is one area where Swift's choice of using signed Int for most things actually works quite well. The equivalent Swift code would be something like (input[safe: i-1] ?? 0) + (input[safe: i+1] ?? 0) though unfortunately Swift doesn't ship with Array.subscript(safe:) so you have to write that yourself.
      - GlitchMr 8y ago
        A slice of a non-ZST element cannot have `usize::MAX` elements, by definition of `usize`.
skookumchuck 8y ago
The only hardcoded numbers in code should be 0 or 1. Anything else, like 5, is a smell.
- quickthrower2 8y ago
  It depends, hard coding 12 as in months in a year is fine. Best to do it through a named constant though.
- nuclx 8y ago
  Depends on the domain.
roywiggins 8y ago
I suppose a side benefit of eliminating special cases is that you also eliminate branches, which may make things faster. If your preprocessing step is quick and branches are very expensive it might make a real difference. It reminds me of programming assignments where you're tasked to solve a maze. Exploring the maze is really annoying if you have to special case the edges; much easier if you just build an edge-of-the-world wall first.
chrismorgan 8y ago
The first example is not at all good. It does not, in fact, eliminate the special cases, and becomes almost certainly less efficient—demonstrating why I would say that for this particular case a Good Programmer would write the code in the first form and not the second. The special cases: they are not removed, merely transformed from `if` statements to boundary conditions on the iterator. The `i == 0` and `i == input.size - 1` checks effectively move into the iterator, with how the nice and clear `input.map.with_index do |_, i|` becomes the magic `(1...padded.size-1).map do |i|`. The 0 added to the start and end are also easily arguably special cases. The second form is almost certainly less efficient because you’re creating at least one new array, unnecessarily. > The special cases are gone, and the simple algorithm shines through. The algorithm does shine through more, but the special cases are not gone, merely transformed (and into an arguably less clear form), and the code is less efficient. Simplifying things is a worthy goal, and a lot of special cases are code smells, but be careful in seeking to remove them, and remember to at least consider performance, even if you end up deciding to prefer succinctness instead.
- ballenf 8y ago
  The repeated if checks inside a loop could easily be less performant than the O(1) operation of adding 2 items to a array. I think you’re too quick to assume that your preferred solution is faster. Personally, I’d use the more clear code and only if the code proved to be a bottleneck even consider refactoring for performance. Whether the author’s preferred solutions are clearer, I’m not convinced however. Finally, the author doesn’t claim that the special cases are gone, but more so that the input is transformed in a way to allow processing that is blind to them.
  - userbinator 8y ago
    than the O(1) operation of adding 2 items to a array. Not very familiar with the language, but the code appears to create a new array by concatenating three arrays, which means copying all the elements; how is that O(1)?
    - falsedan 8y ago
im_down_w_otp 8y ago
I can certainly imagine cases for which this is true, but I more often than not would expect this to be true mostly at the end of a long tail of requirements smell and model smell.
Jorge1o1 8y ago
I disagree with the article. The reasoning is much clearer when you specifically spell out how you’re handling special cases than trying to finagle your input and padding with zeros and whatnot. It’s literally hiding your intentions and reasoning for doing things, which I think is anathema to good code maintenance.
fenomas 8y ago
I'm profoundly unconvinced by this. "Handle special cases, then do the thing" is such a widespread, nigh-universal pattern that I don't think it hides anything from the programmer. It's the pattern used by any function that checks its inputs! Rejiggering an algorithm so as not to have special cases just puts cognitive load on whoever reads the code to suss out what the original algorithm was. "Why does this algorithm require its input array to be padded with zeroes? Oh I see, it doesn't - the developer just did that to remove some if statements."
- westoncb 8y ago
  > Rejiggering an algorithm so as not to have special cases just puts cognitive load on whoever reads the code to suss out what the original algorithm was. Sometimes that's how it goes, but other times the special cases were a result of just not knowing the original problem well enough to think of a clean solution that really captures its essence, and the special cases are evidence of a sort of resultant duct taping. Agreed though, what you're describing definitely happens and I'm not a fan.
- userbinator 8y ago
  Indeed, in the first example the special cases should really be moved out of the loop too, so you have the code looking like this: - code for left edge - code for body - code for right edge Now, both you and the machine don't have to keep thinking about the edge cases every time through the loop body. It's a win-win situation.
  - elephantum 8y ago
    It means you have two extra special cases: 1) list is empty; 2) left edge is the right edge (I.e. length is 1)
    - riskable 8y ago
      Nothing that can't be solved with more conditionals! Conditionals are great because they're clear and concise... if not list: # Can't do anything with this return # or raise error if len(list) < 3: # Ditto return #ditto Sure, there's more code but it's not bad code. It's just straightforward stuff that's easy to read and debug (if necessary).
ruperp 8y ago
You are a code smell.
lucio 8y ago
Isn't the "better" code wrong in the first example? instead of "result = (1...padded.size-1).map do |i|" shouldn't be "result = (1...padded.size-2).map do |i|"?
- bhrgunatha 8y ago
  There are 2 forms of range in ruby: inclusive 2 dots - 1..5 => 1, 2, 3, 4, 5 and exclusive 3 dots 1...5 => 1, 2, 3, 4 It's always felt deeply counter-intuitive to me because .. feels like it represents less than ... but that's a separate discussion.
bfung 8y ago
Arrays are not special cases. They have a beginning and an end. Trying to expand the Array by prepending and appending to it shows lack of knowledge of memory, performance, and maintainability. If loop is refactored away from 'padded', all hell breaks loose 3 months down the line.
i_phish_cats 8y ago
Special cases are a sign you have a business problem that is worth solving.
- swagasaurus-rex 8y ago
  What if the business is a special case among general solution businesses?
hasahmed 8y ago
Got an easy problem to solve? Let's make it difficult to reason about and hard to maintain. But no if statements!
openloop 8y ago
Human beings are special cases.
SatvikBeri 8y ago
I don't think this article captures the main advantage – when you have layers of logic, eliminating special cases lets you have O(n) pieces of code instead of O(n^2). And even if each portion is less readable, 100 functions beats 10,000 functions any day. A good example of this is making values non-nullable by default, with Optional types, and eliminating a huge swath of special case handling. But the benefit doesn't show up without significant scale, and isn't worth it for functions that are just used once or twice.
edflsafoiewq 8y ago
result = [0] * len(xs) for i in range(0, len(xs) - 1): result[i] += xs[i + 1] result[i + 1] += xs[i] return result
freetime2 8y ago
I agree with the principle. Often when you run into a lot of special cases it can be a sign you have modeled something incorrectly. But I don't really agree with these examples. In these cases the "naive" approaches all feel perfectly adequate. Write it out as straightforwardly as possible. Add some unit tests for the special cases. And then move on to something more worthy of your time than trying eliminate a few if statements.
bitwize 8y ago
No, special cases are what happens when nice neat abstractions meet dirty, messy reality. Get used to it, kid.
overgard 8y ago
If you were to translate these examples into a systems programming language like C, I think you’d find these “simpler” cases are anything but. They’d look much more complicated. They’re filled with new allocations and memory copying. They’re essentially new algorithms at that point.
toolslive 8y ago
One of my more painful anecdotes goes like this: Years ago, we were coding a piece of government regulation that decides eligibility of some kind. The rules went something like this "if it's a man and over 60 or over 55 and part of these classes of occupation but not in .. or ... then .... If otoh, it's a woman and ... ". The law was written down in the exact same wording. We coded it in the same way 12 pages of conditionals split out into different functions to make it more readable. Then I came, armed with my knowledge of algebra, Cayley tables and optimization of boolean functions. I created a truth table for the boolean function at hand, optimized that function into a small equivalent expression and replaced the 12 pages of code by a one pager. A job well done. Two months later the law changed adding a new special case somewhere deep inside. I could start over. Lesson learned: don't outsmart the logic of the business.
- baddox 8y ago
  That’s a genuinely great anecdote, and very instructive. Please tell me you used a Karnaugh map.
  - toolslive 8y ago
    yup.
- mrweasel 8y ago
  >Lesson learned: don't outsmart the logic of the business. Very often when people try to solve problems, like the one you faced, with software, they fail to recognize that you might have to rethink the business logic. It's my belief that the majority of software design for specific niche business and governments fail because there was a lack of willingness to change and simplify the rules. Of cause a project create a new system for taxation will fail, if you have 6000 pages of rules, not including the tax law itself. Often we think we have a software bug/issue, in reality what we have is a flawed business logic. Until such time the people in charge fixes the logic, we're not able to do anything clever of efficient in software.
  - toolslive 8y ago
_pmf_ 8y ago
You just change the way they are handled; the knowledge about the special case is now hidden in the setup code, which might be better in some cases and worse in other cases.
ridiculous_fish 8y ago
Hey! Here's a fun trick I learned from Knuth's TAOCP, the one with the tape drive algorithms. You want to search an array for an element: int i; for (i=0; i < array.length; i++) { if (array[i] == value) break; } return i; This is bad because we have two comparisons every loop iteration. But simply append our search term: array.push(value); int i; for (i=0; ; i++) { if (array[i] == value) break; } array.pop(); return i; and we've cut our comparisons in half!
- pvorb 8y ago
  I'd prefer the first version in almost all situations except for a major performance bottle neck. The second version is less clear to the reader and thus likely done wrong.
  - ridiculous_fish 8y ago
    Yeah tape-based algorithms are an anachronism, but the idea of eliminating bounds checking via a terminal value is very powerful. An example of this in action is LLVM's MemoryBuffer [1], which is input to a parser. How to write a parser? Perhaps it performs bounds checking at each production. However LLVM guarantees that the MemoryBuffer is NUL (0) terminated. A parser can then arrange for NUL to be a terminating character, and handle it uniformly. The resulting parsers are faster, and also significantly clearer and easier to write, because there's no need for bounds checking at all. http://llvm.org/doxygen/classllvm_1_1MemoryBuffer.html http://llvm.org/doxygen/classllvm_1_1MemoryBuffer.html
    - magicalhippo 8y ago
      However it requires that the terminal value does not occur in the input. For a parser that's probably not a problem (it can transform NULs in the input to a different token), in other situations it's not so easy.
- brisance 8y ago
  Don't most modern compilers in the last 20 years do this for you automatically?
ridiculous_fish 8y ago
For neighbor-sum, instead of saying "each output value is the sum of its input neighbors", we can say "each input value contributes to the output values of its neighbors." Then it is natural to iterate over the input and not the output: for (int i=1; i < length; i++) output[i-1] += input[i]; for (int i=0; i + 1 < length; i++) output[i+1] += input[i]; "Invert the problem" is a really powerful general strategy.
- pure-awesome 8y ago
  That's actually the kind of solution I thought the author of the article was going to suggest, before I read their solution. On an unrelated note, why do you use i+1<length rather than i<length-1 ?
- deleted 8y ago
  [deleted]
scarejunba 8y ago
Funny. Linus Torvalds somewhat feels this way judging by this https://m.slashdot.org/story/176163 https://m.slashdot.org/story/176163 Repeated below: At the opposite end of the spectrum, I actually wish more people understood the really core low-level kind of coding. Not big, complex stuff like the lockless name lookup, but simply good use of pointers-to-pointers etc. For example, I've seen too many people who delete a singly-linked list entry by keeping track of the "prev" entry, and then to delete the entry, doing something like if (prev) prev->next = entry->next; else list_head = entry->next; and whenever I see code like that, I just go "This person doesn't understand pointers". And it's sadly quite common. People who understand pointers just use a "pointer to the entry pointer", and initialize that with the address of the list_head. And then as they traverse the list, they can remove the entry without using any conditionals, by just doing a "*pp = entry->next".
JoeSmithson 8y ago
This is something I've definitely changed my mind on several times in my life. I studied Maths at uni so I definitely have a bias to the author's position of transforming the problem into a form where an elegant solution "falls out" naturally. However, I think this is because I am comfortable (from training) with transformation steps and can easily see whether they matter or not in terms of affecting the answer. The example the author uses seems nice because their entire audience is experienced/intelligence enough to see why padding zeroes works. There's a puzzle where you are given an array of ints and told every term appears twice except one. You have to find this "lonely element". This can be solved naively by looping through the list and recording how many times you see each number. It can also be solved "elegantly" by XORing the whole list - but it is not immediately obvious why this works. In this case I would definitely say the elegance is not worth the obfuscation cost. The personal compromise I've come to is to try and refactor the code so that specifical cases are extracted and quarantined as much as possible, leaving the main algorithm hopefully clear and elegant. For example, in the article's first example, I would not pad the array but would split out a separate function neighbours(i) which returns [1], [n-1] or [n-1, n+1]. This approach makes more sense in more complicated examples, (say a 3D grid) or in particular if padding is impossible, like you have to calculate (sum of neighbours) + (product of neighbours).
oneeyedpigeon 8y ago
I note that some languages effectively do the array-padding by default. In PHP, accessing an out-of-bounds element results in a Notice (not even a Warning, let alone an Error) and a NULL return value. Of course, in an integer context, NULL gets coerced to 0, so the following will work just fine: $input = array(1, 3, 4, 5, 0, 1, 8); $i = 0; echo $input[$i - 1] + $input[$i + 1]; // outputs 3
0xBA5ED 8y ago
Yes, it would be nice if your inputs could take any form at any time to accommodate your "ideal algorithms". Sadly, inputs have definite form and changing them may require unnecessary work, such as allocating memory!
mokkaweli 8y ago
Life is made of special cases and it isn't an ideal mathematical model that you can describe with a beautiful one liner. Write solid tests, write meaningful comments, don't over engineer things and you should be good.
AstralStorm 8y ago
Can we get the months to be evenly 30 days? ;)
koliber 8y ago
Be smart, write DRY code, but leave room for hooks to implement special cases. Make code readable, maintainable, and extensible. Write for maintainability and extensibility. I know you can solve the given problem better, with more succinct syntax and clever shortcuts. Please don't though. The code you write today is here to stay. Plan for the war, and don't overoptimize the individual battles.
nadam 8y ago
>Before going on, consider how you’d solve this yourself. var sum = 0; for(v : vals) {sum += 2*v;} sum -= (vals.first + vals.last); simpler and much faster than any of the mentioned methods in the article. (Proof: when you sum the neighbours of all the elements then you add each element twice except the first and the last one.) Even simpler and even faster: var s = sum(vals) * 2 - vals.first - vals.last;
- jmmcd 8y ago
  Even though the wording is a bit ambiguous, the problem is to output an array of neighbour-sums, not a single sum-of-neighbour-sums.
  - nadam 8y ago
    oh, ok, thanks.
kurtisc 8y ago
12am/12pm are ambiguous and should be avoided when possible. Try asking people what time 12pm means and you'll see that some will struggle and some will get it wrong, especially if they're from a place that uses a 24 hour clock. But even a 24 hour clock is ambiguous at 00:00. What date should it be? This can be solved by using 11:59 and 23:59 instead. 12pm is usually used for noon. Ironically, of the two choices, it makes the least sense. It makes the PM hours become: 12, 1, 2, 3... When one would expect: ...9, 10, 11, 12 Unfortunately, people weren't quite as into zero-indexing in ancient history.
- scrollaway 8y ago
  Right, 12 is 0 on AM/PM clocks. Once you get to understand that, it's easy to reason with (though 24-hour clocks are the far superior siege weapon). 00:00 is not ambiguous, though. There is no such date on 12-hour clocks (there's no zero! Just 12…), and on 24-hour clocks it's unambiguously the beginning of the day (24-hour clocks are zero-indexed). In fact, 00:* dates are consistently unambiguous, because they don't require you to define whether you're using a 12 or 24 hour clock. I think you're thinking about 12:00 on 12 hour clocks.
DarkWiiPlayer 8y ago
I always wondered how on modern day computers software can still run as slow as it often does. This article has really opened my eyes.
rlv-dan 8y ago
Special cases exists because reality is not perfect.
khendron 8y ago
This is the coding equivalent of a designer putting appearance over usability. In this case, it is putting code elegance over readability/maintainability. Sure, you've reduced the number of if statements, but those if statements explicitly represent the logic behind the calculations. Come back 6 months later, and you'll be scratching your head about what's going on. Unless you add a comment, but comments are also a code smell.
- riskable 8y ago
  Comments are a code smell? WTF? No. Just... No. Not having comments is a code smell. Everything has context and it is wrong to assume that the next developer to come along will know/understand it well (or at all). Even you as the original coder might not remember why you did something a certain way. I personally comment like I'm going to start suffering from a massive cognitive decline any day now and will need most things re-explained to me. Example: I was looking at some old code this morning... temp.write('\n') temp.close() Why's that newline being written there? From the perspective of the code it serves no propose. Good thing I had a comment right above it... # Add a trailing newline so 'cat' doesn't leave an ugly mess "Ah, yes. That's a good reason to add a newline."
  - Retric 8y ago
    You misunderstood. The need for comments means the code is not clear. That need is a code smell.
    - magicalhippo 8y ago
      I think that's too broad. Needing comments to explain what the code does is a smell. Needing comments to explain why the code needs to do what it does is not a smell.
      - Retric 8y ago
        I don’t think zero comments should be the goal, just that minimizing the need for them is often a good idea. Why can generally be included in the code. Total = SubTotal + SalesTax; Needs no real explanation. Code smell is just another way to compare two possible approaches. If one version can include significantly fewer comments without issue then that’s good sign.
nikanj 8y ago
Where there's muck, there's brass. Business is smelly, messy, and just filled with exceptions, special cases, and weird deals with an diabolical structures. Most startups start with a beautiful, elegant system with zero real users, then they hit traction, and with added business the business logic parts of the code base turn into eldritch monstrosities.
LudwigNagasena 8y ago
Ugly structures that don't map to clear concepts are worse than any special case. Padding array makes you go "wtf is that?" I would rather see something like this: (i > 0 ? input[i-1] : 0) + (i < input.size - 1 ? input[i+1] : 0) which clearly maps to "if there is a left neighbor, add it; if there is a right neighbor, add it; return the sum."
austincheney 8y ago
One-offs and edge case requirements are everywhere. The article entertains the idea of eliminating edge cases. Edge case requirements aren't the problem. The problem is how you solve for them. This is a form of complexity management and fortunately there are a few good approaches that solve for most forms of complexity. The way I commonly approach edge cases is that they are dealt with by an additional rule (and possibly accompanying data structure) in otherwise existing similar common functionality. If current functionality is not sufficient then write new functionality to solve for and replace other existing insufficient functionality. This allows for consideration of edge cases as formal requirements while minimizing expansion of complexity. It is refactoring and not innovation. If there is a need for innovation and new functionality then the requirement is a dedicated feature instead of an edge case. The difference between a feature and an edge case is the different level of intentional overhead required for documentation, integration, testing, and maintenance.
gerbilly 8y ago
I used to have a math teacher in high school who would say: "When a mathematician doesn't have what he wants, he re-arranges things so as to get it." Basically he meant, if the problem isn't presented in a way that makes it convenient to solve, rearrange the problem fist, then solve it. A classic example of this from high school math would be: https://en.wikipedia.org/wiki/Completing_the_square https://en.wikipedia.org/wiki/Completing_the_square
- rahimnathwani 8y ago
  "If I need to add a new function and the design does not suit the change, I find it’s quicker to refactor first and then add the function." https://martinfowler.com/books/refactoring.html https://martinfowler.com/books/refactoring.html
csours 8y ago
Sometimes it feels like all I do is move problems around like boiled broccoli on a plate.
uberman 8y ago
Forcing others to alter their inputs because you want to simplify your method (because code smell??) is nuts. If you say: Oh by the way, if you want to call "final_step" be sure to remember to mirror your input array otherwise my method will break. I would say: Oh by the way, You are fired.
taco_emoji 8y ago
This article is unconvincing--the author doesn't even attempt to justify the claims made throughout. I.e.: > Notice how we treat the endpoints as special cases, complicating the simple rule “sum both neighbors of every element.” A better approach is to first transform the input so the special cases vanish, leaving only the general case. Why is that a better approach? The author never deigns to share their reasoning.
taurath 8y ago
I'd say if you're working for a real business making money and don't have special cases in the code, you should watch out because some competitor is going to be moving faster than you, and you probably don't bring much value. In any significantly complex system of business logic, special cases abound. A system of only special cases is of course bad, but a system with a few here and there are expected.
dkarl 8y ago
It's way too strong to say special cases are a code smell. A code smell is something that means most likely something is off. Special cases are just one way of achieving a result and often the most readable and maintainable one. The alternatives can be just as "smelly" in their own right. I'd say this is a special case (ha!) of the principle that each person and organization has their own personal tendencies. If you say "X is a code smell" and the internet disagrees, it probably means you or your organization have a tendency to choose X when it's not the right solution.

198 points by fagnerbrack 8y ago

Certainly subjective. One could in many instances easily find it clearer making the special cases explicit instead of messing with the data beforehand in order to force it to fit into a more general algorithm, these instances included.

PaulHoule 8y ago

The structure of common sense knowledge is "A unless B unless C unless D..." Thus the ultimate way to organize a program is to have a clear separation between the specific and the general.

clusmore 8y ago

I'm reminded of Linus Torvald's interview at Ted [1] where he talks about programming "taste", and compares the following two code snippets: remove_list_entry(entry) { prev = NULL; walk = head; while (walk != entry) { prev = walk; walk = walk->next; } if (!prev) head = entry->next; else prev->next = entry->next; } remove_list_entry(entry) { indirect = &head; while ((*indirect) != entry) indirect = &(*indirect)->next; *indirect = entry->next; } [1]: https://www.youtube.com/watch?v=o8NPllzkFhE https://www.youtube.com/watch?v=o8NPllzkFhE

gumby 8y ago
That first case is hideous (hope Linus agreed with me!) but does remind me of my favorite computer scientist joke: === The physicist is showing his friend the programmer a thermos. "You see, you can put a hot drink inside, and then take it with you. It doesn't matter how cold it gets outside; when you pour the drink out it's still hot!" The programmer is quite impressed. The physicist continues, "Or you can put a cold drink inside, and then take it with you. It doesn't matter how hot it gets outside; when you pour the drink out it's still cold!" Now the programmer is really dumbfounded. He asks, "But how does it know?" === Unfortunately though I like the joke, I still see waaay too much code written by the programmer in the joke.
- foota 8y ago
  Is the joke that the programmer interprets it as the drink waiting to pour out? Or?
  - gumby 8y ago
    A non-programmer will just figure the programmer is a bozo (you could tell this joke with a physicist and a geologist, or any combination of <local-in-group-0> and <local-out-group-0>): that's simply how thermoses work. But a thermos with a switch on the top to say "insert hot" or "insert cold" would be absurd. A thermos is simply a container _with no concept of temperature_, just isolation: it just maintains the state blindly; whether that state is hot or cold is managed by the "caller" who inserts and extracts the contents, inspecting them itself if it wants to know what had been put into it. Yet I still see plenty of code that does have a variable to indicate what's inside, perhaps an enum {hot, cold} or worse a bool. Yuck -- you should simply store the thing and if you need to know its state you look at it. That variable is something to get out of sync; something you need to be careful to set and maintain, something at threat to a race condition or ill-timed interrupt. Bleagh. If your first language was FORTRAN, OK. Otherwise there is no excuse. Sadly I see shit like that in pedagogical examples too, which is malpractice.
    - warent 8y ago
      "I still see plenty of code that does have a variable to indicate what's inside, perhaps an enum {hot, cold} or worse a bool. Yuck -- you should simply store the thing and if you need to know its state you look at it." Now I'm confused... How are those two things different? The variable indicating what's inside is going to be coming from the actual state itself, no? It sounds like perhaps you're talking about some case where a developer has somehow copied a state and referred to the copy somewhere else. Or are you talking about a computed variable of some part of the state being a code smell?
      - AstralStorm 8y ago
        He's talking about explicit state, which can be a code smell when you can have equality efficient (or more) stateless solution.
westoncb 8y ago
This is an especially interesting comparison because while it does clearly exhibit the sort of elegance only achievable through the elimination of special cases, it's also a good demonstration of the tradeoffs one sometimes has to make for this purpose. The second is significantly 'cleaner,' but with pretty bad readability [0]. The first version has excellent readability. In practice, the way I'd evaluate the two is by looking at how much other code depends on (or at some point may depend on) the piece under consideration. If nothing else or only very little depends on the code, I'll opt for readability; if it's likely to be somewhat foundational and other stuff is going to grow on it, I'll opt for the more abstract, cleaner version, and document thoroughly. [0] I'm guessing the readability isn't nearly as bad if the first example is using a common C idiom; I don't use C enough to know. My main criterion for judging readability is: how much does the structure of the code match the conception of the algorithm you're starting with. The conception here has to do with navigating links; the pointer stuff is incidental complexity.
- im3w1l 8y ago
  To me the main readability problem of the first example is that it does the head check as the last step rather then first step. Meaning we have to consider "wait, when is this null again". The main problem with the last one is that it's not immediately obvious what the "indirect" represents. When it clicks that it is "the pointer to potentially update", it's pretty easy to understand.
  - ummonk 8y ago
    The 2nd one would be significantly more readable with explicit typing (to make it clear that you're dealing with a pointer to pointer). Though I think putting the head check at the beginning like you said will make the first example very reasonable (and relatively compact too). remove_list_entry(entry) { if (head == entry) { head = entry->next; } else { prev = head; while (prev->next != entry) { prev = prev->next; } prev->next = entry->next; } }
userbinator 8y ago
There's a third alternative, which I've heard of as "virtual head", that removes the extra indirections in the loop and removal: while(p->next != entry) p = p->next; p->next = entry->next; I've deliberately not shown the initialisation of p, because it's a bit tricky in C (but trivial in Asm); p is not initialised to the head, nor the address of the head, but to a "virtual" node centered around the head, such that p->next will initially access the head. If the next field is at the beginning of the structure, p does point to the head; else p points to a location before the head. It would be something like (char*)&head - offsetof(Node, next);
- DerekL 8y ago
  You can get the same effect by using a “dummy” node. The dummy node has a next pointer, but no data, and the dummy node always exists at the front of the list. The list structure contains the dummy, instead of using a pointer to the front of the list. struct Node { struct Node* next; }; struct DataNode { struct Node* next; struct Value value; }; struct List { struct Node dummy; // You can also add length, etc. }; // entry must be in list void remove_list_entry(struct List* list, struct Node* entry) { Node *prev = &list->dummy; while (prev->next != entry) { prev = prev->next; assert(prev != NULL); } prev->next = entry->next; free(entry); }
- rovolo 8y ago
  For those curious how to create the "virtual head" in C: struct entry { int value; entry * next; }; entry * head = NULL; void remove_entry(entry *target) { long offset = (long)&((entry*)0)->next; entry * curr = (entry*)((long)&head - offset); while (curr->next != target) curr = curr->next; curr->next = target->next; } But I'm not sure how portable that is. It is portable if the next pointer is the first member of the struct though. From 6.7.2.1.13 in the C99 standard: >A pointer to a structure object, suitably converted, points to its initial member struct entry { entry * next; int value; }; ... // virtual entry where &(curr->next) = &head entry * curr = (entry*)&head; Note that it's exactly the same logic as the grandparent since *curr = curr->next indirect = curr *indirect = *curr *indirect = curr->next indirect = &(curr->next) But, I think that the virtual head entry is an easier mental model. C99 standard: http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf

gumby 8y ago

These suggested fixes look worse to me. In the first one, the "wrong" function was quite clear (special case 0, special case 1, general case) while the second one allocated memory. I'd call that a code smell!

eridius 8y ago
Agreed. A better solution here is to have a generic accessor on the Array that can return a default value for out-of-bounds accesses. There almost is one already, the two-argument version of fetch(), except that interprets negative indices as counting from the end. But if we create a variant that doesn't treat negative indices specially then the code becomes very simple result = input.map.with_index do |_, i| input.better_fetch(i-1, 0) + input.better_fetch(i+1, 0) end.to_a
- chrismorgan 8y ago
  Yeah, in Rust it’d fall very neatly out of the type system, as `input.get(i - 1).unwrap_or(0) + input.get(i + 1).unwrap_or(0)`.
  - eridius 8y ago
    Funnily enough, that Rust code is actually buggy, because i is a usize and if i == 0 then i-1 will panic due to underflow. You'd have to write input.get(i.wrapping_sub(1)).unwrap_or(0) + input.get(i+1).unwrap_or(0) This also technically won't work right if the slice has usize::MAX elements, but that's rather unlikely. This is one area where Swift's choice of using signed Int for most things actually works quite well. The equivalent Swift code would be something like (input[safe: i-1] ?? 0) + (input[safe: i+1] ?? 0) though unfortunately Swift doesn't ship with Array.subscript(safe:) so you have to write that yourself.
    - GlitchMr 8y ago
      A slice of a non-ZST element cannot have `usize::MAX` elements, by definition of `usize`.

skookumchuck 8y ago

The only hardcoded numbers in code should be 0 or 1. Anything else, like 5, is a smell.

quickthrower2 8y ago
It depends, hard coding 12 as in months in a year is fine. Best to do it through a named constant though.
nuclx 8y ago
Depends on the domain.

roywiggins 8y ago

I suppose a side benefit of eliminating special cases is that you also eliminate branches, which may make things faster. If your preprocessing step is quick and branches are very expensive it might make a real difference. It reminds me of programming assignments where you're tasked to solve a maze. Exploring the maze is really annoying if you have to special case the edges; much easier if you just build an edge-of-the-world wall first.

chrismorgan 8y ago

The first example is not at all good. It does not, in fact, eliminate the special cases, and becomes almost certainly less efficient—demonstrating why I would say that for this particular case a Good Programmer would write the code in the first form and not the second. The special cases: they are not removed, merely transformed from `if` statements to boundary conditions on the iterator. The `i == 0` and `i == input.size - 1` checks effectively move into the iterator, with how the nice and clear `input.map.with_index do |_, i|` becomes the magic `(1...padded.size-1).map do |i|`. The 0 added to the start and end are also easily arguably special cases. The second form is almost certainly less efficient because you’re creating at least one new array, unnecessarily. > The special cases are gone, and the simple algorithm shines through. The algorithm does shine through more, but the special cases are not gone, merely transformed (and into an arguably less clear form), and the code is less efficient. Simplifying things is a worthy goal, and a lot of special cases are code smells, but be careful in seeking to remove them, and remember to at least consider performance, even if you end up deciding to prefer succinctness instead.

ballenf 8y ago
The repeated if checks inside a loop could easily be less performant than the O(1) operation of adding 2 items to a array. I think you’re too quick to assume that your preferred solution is faster. Personally, I’d use the more clear code and only if the code proved to be a bottleneck even consider refactoring for performance. Whether the author’s preferred solutions are clearer, I’m not convinced however. Finally, the author doesn’t claim that the special cases are gone, but more so that the input is transformed in a way to allow processing that is blind to them.
- userbinator 8y ago
  than the O(1) operation of adding 2 items to a array. Not very familiar with the language, but the code appears to create a new array by concatenating three arrays, which means copying all the elements; how is that O(1)?
  - falsedan 8y ago

im_down_w_otp 8y ago

I can certainly imagine cases for which this is true, but I more often than not would expect this to be true mostly at the end of a long tail of requirements smell and model smell.

Jorge1o1 8y ago

I disagree with the article. The reasoning is much clearer when you specifically spell out how you’re handling special cases than trying to finagle your input and padding with zeros and whatnot. It’s literally hiding your intentions and reasoning for doing things, which I think is anathema to good code maintenance.

fenomas 8y ago

I'm profoundly unconvinced by this. "Handle special cases, then do the thing" is such a widespread, nigh-universal pattern that I don't think it hides anything from the programmer. It's the pattern used by any function that checks its inputs! Rejiggering an algorithm so as not to have special cases just puts cognitive load on whoever reads the code to suss out what the original algorithm was. "Why does this algorithm require its input array to be padded with zeroes? Oh I see, it doesn't - the developer just did that to remove some if statements."

westoncb 8y ago
> Rejiggering an algorithm so as not to have special cases just puts cognitive load on whoever reads the code to suss out what the original algorithm was. Sometimes that's how it goes, but other times the special cases were a result of just not knowing the original problem well enough to think of a clean solution that really captures its essence, and the special cases are evidence of a sort of resultant duct taping. Agreed though, what you're describing definitely happens and I'm not a fan.
userbinator 8y ago
Indeed, in the first example the special cases should really be moved out of the loop too, so you have the code looking like this: - code for left edge - code for body - code for right edge Now, both you and the machine don't have to keep thinking about the edge cases every time through the loop body. It's a win-win situation.
- elephantum 8y ago
  It means you have two extra special cases: 1) list is empty; 2) left edge is the right edge (I.e. length is 1)
  - riskable 8y ago
    Nothing that can't be solved with more conditionals! Conditionals are great because they're clear and concise... if not list: # Can't do anything with this return # or raise error if len(list) < 3: # Ditto return #ditto Sure, there's more code but it's not bad code. It's just straightforward stuff that's easy to read and debug (if necessary).

ruperp 8y ago

You are a code smell.

lucio 8y ago

Isn't the "better" code wrong in the first example? instead of "result = (1...padded.size-1).map do |i|" shouldn't be "result = (1...padded.size-2).map do |i|"?

bhrgunatha 8y ago
There are 2 forms of range in ruby: inclusive 2 dots - 1..5 => 1, 2, 3, 4, 5 and exclusive 3 dots 1...5 => 1, 2, 3, 4 It's always felt deeply counter-intuitive to me because .. feels like it represents less than ... but that's a separate discussion.

bfung 8y ago

Arrays are not special cases. They have a beginning and an end. Trying to expand the Array by prepending and appending to it shows lack of knowledge of memory, performance, and maintainability. If loop is refactored away from 'padded', all hell breaks loose 3 months down the line.

i_phish_cats 8y ago

Special cases are a sign you have a business problem that is worth solving.

swagasaurus-rex 8y ago
What if the business is a special case among general solution businesses?

hasahmed 8y ago

Got an easy problem to solve? Let's make it difficult to reason about and hard to maintain. But no if statements!

openloop 8y ago

Human beings are special cases.

SatvikBeri 8y ago

I don't think this article captures the main advantage – when you have layers of logic, eliminating special cases lets you have O(n) pieces of code instead of O(n^2). And even if each portion is less readable, 100 functions beats 10,000 functions any day. A good example of this is making values non-nullable by default, with Optional types, and eliminating a huge swath of special case handling. But the benefit doesn't show up without significant scale, and isn't worth it for functions that are just used once or twice.

edflsafoiewq 8y ago

result = [0] * len(xs) for i in range(0, len(xs) - 1): result[i] += xs[i + 1] result[i + 1] += xs[i] return result

freetime2 8y ago

I agree with the principle. Often when you run into a lot of special cases it can be a sign you have modeled something incorrectly. But I don't really agree with these examples. In these cases the "naive" approaches all feel perfectly adequate. Write it out as straightforwardly as possible. Add some unit tests for the special cases. And then move on to something more worthy of your time than trying eliminate a few if statements.

bitwize 8y ago

No, special cases are what happens when nice neat abstractions meet dirty, messy reality. Get used to it, kid.

overgard 8y ago

If you were to translate these examples into a systems programming language like C, I think you’d find these “simpler” cases are anything but. They’d look much more complicated. They’re filled with new allocations and memory copying. They’re essentially new algorithms at that point.

toolslive 8y ago

One of my more painful anecdotes goes like this: Years ago, we were coding a piece of government regulation that decides eligibility of some kind. The rules went something like this "if it's a man and over 60 or over 55 and part of these classes of occupation but not in .. or ... then .... If otoh, it's a woman and ... ". The law was written down in the exact same wording. We coded it in the same way 12 pages of conditionals split out into different functions to make it more readable. Then I came, armed with my knowledge of algebra, Cayley tables and optimization of boolean functions. I created a truth table for the boolean function at hand, optimized that function into a small equivalent expression and replaced the 12 pages of code by a one pager. A job well done. Two months later the law changed adding a new special case somewhere deep inside. I could start over. Lesson learned: don't outsmart the logic of the business.

baddox 8y ago
That’s a genuinely great anecdote, and very instructive. Please tell me you used a Karnaugh map.
- toolslive 8y ago
  yup.
mrweasel 8y ago
>Lesson learned: don't outsmart the logic of the business. Very often when people try to solve problems, like the one you faced, with software, they fail to recognize that you might have to rethink the business logic. It's my belief that the majority of software design for specific niche business and governments fail because there was a lack of willingness to change and simplify the rules. Of cause a project create a new system for taxation will fail, if you have 6000 pages of rules, not including the tax law itself. Often we think we have a software bug/issue, in reality what we have is a flawed business logic. Until such time the people in charge fixes the logic, we're not able to do anything clever of efficient in software.
- toolslive 8y ago

_pmf_ 8y ago

You just change the way they are handled; the knowledge about the special case is now hidden in the setup code, which might be better in some cases and worse in other cases.

ridiculous_fish 8y ago

Hey! Here's a fun trick I learned from Knuth's TAOCP, the one with the tape drive algorithms. You want to search an array for an element: int i; for (i=0; i < array.length; i++) { if (array[i] == value) break; } return i; This is bad because we have two comparisons every loop iteration. But simply append our search term: array.push(value); int i; for (i=0; ; i++) { if (array[i] == value) break; } array.pop(); return i; and we've cut our comparisons in half!

pvorb 8y ago
I'd prefer the first version in almost all situations except for a major performance bottle neck. The second version is less clear to the reader and thus likely done wrong.
- ridiculous_fish 8y ago
  Yeah tape-based algorithms are an anachronism, but the idea of eliminating bounds checking via a terminal value is very powerful. An example of this in action is LLVM's MemoryBuffer [1], which is input to a parser. How to write a parser? Perhaps it performs bounds checking at each production. However LLVM guarantees that the MemoryBuffer is NUL (0) terminated. A parser can then arrange for NUL to be a terminating character, and handle it uniformly. The resulting parsers are faster, and also significantly clearer and easier to write, because there's no need for bounds checking at all. http://llvm.org/doxygen/classllvm_1_1MemoryBuffer.html http://llvm.org/doxygen/classllvm_1_1MemoryBuffer.html
  - magicalhippo 8y ago
    However it requires that the terminal value does not occur in the input. For a parser that's probably not a problem (it can transform NULs in the input to a different token), in other situations it's not so easy.
brisance 8y ago
Don't most modern compilers in the last 20 years do this for you automatically?

ridiculous_fish 8y ago

For neighbor-sum, instead of saying "each output value is the sum of its input neighbors", we can say "each input value contributes to the output values of its neighbors." Then it is natural to iterate over the input and not the output: for (int i=1; i < length; i++) output[i-1] += input[i]; for (int i=0; i + 1 < length; i++) output[i+1] += input[i]; "Invert the problem" is a really powerful general strategy.

pure-awesome 8y ago
That's actually the kind of solution I thought the author of the article was going to suggest, before I read their solution. On an unrelated note, why do you use i+1<length rather than i<length-1 ?
deleted 8y ago
[deleted]

scarejunba 8y ago

Funny. Linus Torvalds somewhat feels this way judging by this https://m.slashdot.org/story/176163 https://m.slashdot.org/story/176163 Repeated below: At the opposite end of the spectrum, I actually wish more people understood the really core low-level kind of coding. Not big, complex stuff like the lockless name lookup, but simply good use of pointers-to-pointers etc. For example, I've seen too many people who delete a singly-linked list entry by keeping track of the "prev" entry, and then to delete the entry, doing something like if (prev) prev->next = entry->next; else list_head = entry->next; and whenever I see code like that, I just go "This person doesn't understand pointers". And it's sadly quite common. People who understand pointers just use a "pointer to the entry pointer", and initialize that with the address of the list_head. And then as they traverse the list, they can remove the entry without using any conditionals, by just doing a "*pp = entry->next".

JoeSmithson 8y ago

This is something I've definitely changed my mind on several times in my life. I studied Maths at uni so I definitely have a bias to the author's position of transforming the problem into a form where an elegant solution "falls out" naturally. However, I think this is because I am comfortable (from training) with transformation steps and can easily see whether they matter or not in terms of affecting the answer. The example the author uses seems nice because their entire audience is experienced/intelligence enough to see why padding zeroes works. There's a puzzle where you are given an array of ints and told every term appears twice except one. You have to find this "lonely element". This can be solved naively by looping through the list and recording how many times you see each number. It can also be solved "elegantly" by XORing the whole list - but it is not immediately obvious why this works. In this case I would definitely say the elegance is not worth the obfuscation cost. The personal compromise I've come to is to try and refactor the code so that specifical cases are extracted and quarantined as much as possible, leaving the main algorithm hopefully clear and elegant. For example, in the article's first example, I would not pad the array but would split out a separate function neighbours(i) which returns [1], [n-1] or [n-1, n+1]. This approach makes more sense in more complicated examples, (say a 3D grid) or in particular if padding is impossible, like you have to calculate (sum of neighbours) + (product of neighbours).

oneeyedpigeon 8y ago

I note that some languages effectively do the array-padding by default. In PHP, accessing an out-of-bounds element results in a Notice (not even a Warning, let alone an Error) and a NULL return value. Of course, in an integer context, NULL gets coerced to 0, so the following will work just fine: $input = array(1, 3, 4, 5, 0, 1, 8); $i = 0; echo $input[$i - 1] + $input[$i + 1]; // outputs 3

0xBA5ED 8y ago

Yes, it would be nice if your inputs could take any form at any time to accommodate your "ideal algorithms". Sadly, inputs have definite form and changing them may require unnecessary work, such as allocating memory!

mokkaweli 8y ago

Life is made of special cases and it isn't an ideal mathematical model that you can describe with a beautiful one liner. Write solid tests, write meaningful comments, don't over engineer things and you should be good.

AstralStorm 8y ago

Can we get the months to be evenly 30 days? ;)

koliber 8y ago

Be smart, write DRY code, but leave room for hooks to implement special cases. Make code readable, maintainable, and extensible. Write for maintainability and extensibility. I know you can solve the given problem better, with more succinct syntax and clever shortcuts. Please don't though. The code you write today is here to stay. Plan for the war, and don't overoptimize the individual battles.

nadam 8y ago

>Before going on, consider how you’d solve this yourself. var sum = 0; for(v : vals) {sum += 2*v;} sum -= (vals.first + vals.last); simpler and much faster than any of the mentioned methods in the article. (Proof: when you sum the neighbours of all the elements then you add each element twice except the first and the last one.) Even simpler and even faster: var s = sum(vals) * 2 - vals.first - vals.last;

jmmcd 8y ago
Even though the wording is a bit ambiguous, the problem is to output an array of neighbour-sums, not a single sum-of-neighbour-sums.
- nadam 8y ago
  oh, ok, thanks.

kurtisc 8y ago

12am/12pm are ambiguous and should be avoided when possible. Try asking people what time 12pm means and you'll see that some will struggle and some will get it wrong, especially if they're from a place that uses a 24 hour clock. But even a 24 hour clock is ambiguous at 00:00. What date should it be? This can be solved by using 11:59 and 23:59 instead. 12pm is usually used for noon. Ironically, of the two choices, it makes the least sense. It makes the PM hours become: 12, 1, 2, 3... When one would expect: ...9, 10, 11, 12 Unfortunately, people weren't quite as into zero-indexing in ancient history.

scrollaway 8y ago
Right, 12 is 0 on AM/PM clocks. Once you get to understand that, it's easy to reason with (though 24-hour clocks are the far superior siege weapon). 00:00 is not ambiguous, though. There is no such date on 12-hour clocks (there's no zero! Just 12…), and on 24-hour clocks it's unambiguously the beginning of the day (24-hour clocks are zero-indexed). In fact, 00:* dates are consistently unambiguous, because they don't require you to define whether you're using a 12 or 24 hour clock. I think you're thinking about 12:00 on 12 hour clocks.

DarkWiiPlayer 8y ago

I always wondered how on modern day computers software can still run as slow as it often does. This article has really opened my eyes.

rlv-dan 8y ago

Special cases exists because reality is not perfect.

khendron 8y ago

This is the coding equivalent of a designer putting appearance over usability. In this case, it is putting code elegance over readability/maintainability. Sure, you've reduced the number of if statements, but those if statements explicitly represent the logic behind the calculations. Come back 6 months later, and you'll be scratching your head about what's going on. Unless you add a comment, but comments are also a code smell.

riskable 8y ago
Comments are a code smell? WTF? No. Just... No. Not having comments is a code smell. Everything has context and it is wrong to assume that the next developer to come along will know/understand it well (or at all). Even you as the original coder might not remember why you did something a certain way. I personally comment like I'm going to start suffering from a massive cognitive decline any day now and will need most things re-explained to me. Example: I was looking at some old code this morning... temp.write('\n') temp.close() Why's that newline being written there? From the perspective of the code it serves no propose. Good thing I had a comment right above it... # Add a trailing newline so 'cat' doesn't leave an ugly mess "Ah, yes. That's a good reason to add a newline."
- Retric 8y ago
  You misunderstood. The need for comments means the code is not clear. That need is a code smell.
  - magicalhippo 8y ago
    I think that's too broad. Needing comments to explain what the code does is a smell. Needing comments to explain why the code needs to do what it does is not a smell.
    - Retric 8y ago
      I don’t think zero comments should be the goal, just that minimizing the need for them is often a good idea. Why can generally be included in the code. Total = SubTotal + SalesTax; Needs no real explanation. Code smell is just another way to compare two possible approaches. If one version can include significantly fewer comments without issue then that’s good sign.

nikanj 8y ago

Where there's muck, there's brass. Business is smelly, messy, and just filled with exceptions, special cases, and weird deals with an diabolical structures. Most startups start with a beautiful, elegant system with zero real users, then they hit traction, and with added business the business logic parts of the code base turn into eldritch monstrosities.

LudwigNagasena 8y ago

Ugly structures that don't map to clear concepts are worse than any special case. Padding array makes you go "wtf is that?" I would rather see something like this: (i > 0 ? input[i-1] : 0) + (i < input.size - 1 ? input[i+1] : 0) which clearly maps to "if there is a left neighbor, add it; if there is a right neighbor, add it; return the sum."

austincheney 8y ago

One-offs and edge case requirements are everywhere. The article entertains the idea of eliminating edge cases. Edge case requirements aren't the problem. The problem is how you solve for them. This is a form of complexity management and fortunately there are a few good approaches that solve for most forms of complexity. The way I commonly approach edge cases is that they are dealt with by an additional rule (and possibly accompanying data structure) in otherwise existing similar common functionality. If current functionality is not sufficient then write new functionality to solve for and replace other existing insufficient functionality. This allows for consideration of edge cases as formal requirements while minimizing expansion of complexity. It is refactoring and not innovation. If there is a need for innovation and new functionality then the requirement is a dedicated feature instead of an edge case. The difference between a feature and an edge case is the different level of intentional overhead required for documentation, integration, testing, and maintenance.

gerbilly 8y ago

I used to have a math teacher in high school who would say: "When a mathematician doesn't have what he wants, he re-arranges things so as to get it." Basically he meant, if the problem isn't presented in a way that makes it convenient to solve, rearrange the problem fist, then solve it. A classic example of this from high school math would be: https://en.wikipedia.org/wiki/Completing_the_square https://en.wikipedia.org/wiki/Completing_the_square

rahimnathwani 8y ago
"If I need to add a new function and the design does not suit the change, I find it’s quicker to refactor first and then add the function." https://martinfowler.com/books/refactoring.html https://martinfowler.com/books/refactoring.html

csours 8y ago

Sometimes it feels like all I do is move problems around like boiled broccoli on a plate.

uberman 8y ago

Forcing others to alter their inputs because you want to simplify your method (because code smell??) is nuts. If you say: Oh by the way, if you want to call "final_step" be sure to remember to mirror your input array otherwise my method will break. I would say: Oh by the way, You are fired.

taco_emoji 8y ago

This article is unconvincing--the author doesn't even attempt to justify the claims made throughout. I.e.: > Notice how we treat the endpoints as special cases, complicating the simple rule “sum both neighbors of every element.” A better approach is to first transform the input so the special cases vanish, leaving only the general case. Why is that a better approach? The author never deigns to share their reasoning.

taurath 8y ago

I'd say if you're working for a real business making money and don't have special cases in the code, you should watch out because some competitor is going to be moving faster than you, and you probably don't bring much value. In any significantly complex system of business logic, special cases abound. A system of only special cases is of course bad, but a system with a few here and there are expected.

dkarl 8y ago

It's way too strong to say special cases are a code smell. A code smell is something that means most likely something is off. Special cases are just one way of achieving a result and often the most readable and maintainable one. The alternatives can be just as "smelly" in their own right. I'd say this is a special case (ha!) of the principle that each person and organization has their own personal tendencies. If you say "X is a code smell" and the internet disagrees, it probably means you or your organization have a tendency to choose X when it's not the right solution.