2 is a code smell

http://tech.puredanger.com/2011/10/12/2-is-a-smell/

by puredanger • 15 years ago

73 30 15 years ago

tech.puredanger.com

imurray• 15 years ago

Reminds me of zip, zip3, zip4, etc. in Haskell.

ams6110• 15 years ago

Erlang as well, though they don't go to 4 IIRC. A pragmatic compromise of providing a simpler interface for common use cases.

nemetroid• 15 years ago

Haskell actually goes all the way up to zip7. A generalised zipn function is quite complicated to write in Haskell, due to the type system. Here's a blog post about it: http://paczesiowa.blogspot.com/2010/03/generalized-zipwithn....

Jach2• 15 years ago

Cache: http://webcache.googleusercontent.com/search?q=cache%3Ahttp%...

tl;dr Clojure is awesome. And use

    (defn add-vectors [& vs]
     (vec (apply map + vs)))

instead of

    (defn add-vectors [va vb]
     [(+ (first va) (first vb)) (+ (second va) (second vb))])

since in Clojure code that operates on 1 or N things is usually better than some other fixed value. (Like 2.)

KeithMajhor• 15 years ago

What about sub-vectors? That smells like a 2 to me...

I'd rather call fold or foldr myself and KNOW how it handled more than 2 arguments. I'd rather do it for add-vectors too, just so it was consistent. You have to define primitives somewhere. Even if what you call elsewhere is some fluffy handle-everything function. At the very least, I wouldn't call it a smell.

cube13• 15 years ago

I agree that it isn't necessarily a smell.

It really depends on the scope of the problem. Is this supposed to be a fully-functional addition function, or is this a throwaway helper function that the coder wrote because they were doing that specific operation many times, and they didn't want to type the entire thing out? If it's the first case, then it may be an issue. If it's the second, it's just poorly named, and not necessarily an issue at all.

narain• 15 years ago

I think there's a distinction that needs to be drawn here between data that only has two of something, and functions that only take two arguments. In the former case, you limit the things you can represent and generalize to, and everybody agrees that's not good. In the latter, though, there is the standard notion of folding that naturally extends your binary function to arbitrarily many arguments. In fact, I'd agree with what I think you're suggesting: it's better to define a binary function and let fold generalize it, than to have to roll your own multi-argument function every time, because then you and anyone else looking at it can easily know how it behaves. In other words, I think he got carried away by the end of the article, and his last "improvement" isn't an improvement at all.

(Of course, the objection to data having only two things doesn't hold either if it's a recursive data type -- cons lists, binary trees, etc... -- but you know what I mean.)

heyrhett• 15 years ago

If a co-worker told me that my function was "code smell" because it took 2 arguments, I would tell him that he needs more important things to worry about.

humbledrone3• 15 years ago

If a coworker told me the same thing, I'd ask, "Why?" If they gave me the explanation given in the article, I'd say, "Thank you, that's a nice little improvement. The code is simpler AND more general." Then, I'd go on working, a little bit better of a programmer for it. If I repeated this ritual a few hundred times, I might actually become a halfway-decent programmer. In this field, details matter.

erydo• 15 years ago

There's a difference between seeing that something can be generalized, optimized, or improved and spending the time to do it.

Knowing that difference and applying judgement is a major differentiator between inexperienced programmers and experienced ones. That, I think, is what heyrhett was getting at.

Sharlin• 15 years ago

This is pretty much an instance of the good old "zero, one, infinity" rule.

http://www.catb.org/jargon/html/Z/Zero-One-Infinity-Rule.htm...

brlewis2• 15 years ago

This article restricts itself to a context where following "zero, one, infinity" results in simpler, more concise code. This is a good restriction. In general, following "zero, one, infinity" will often put you at odds with YAGNI.

puredanger2• 15 years ago

My experience with Clojure has been that sticking to N is so harmonious with the Clojure way that it is simpler and yields more flexible code.

If the code is simpler then I don't see how YAGNI comes into play. Why would you do something that is both more complicated and less amenable to future needs?

fhars• 15 years ago

You might with more justification claim that varargs methods are a code smell, as they conflate the algebraic structure of the elements and the operations on them (for example that they form a semigroup with respect to the operation at hand) with the traversal over a collection of elements, thereby grossly violationg the principle of separation of concerns. After all, many interesting algebraic structures are defined in terms of binary operators. If you separate operations between elements and traversal over collections, you can fold any operation over any traversable entity, with just #operations + #traversables functions, instead of #operations * #traversables functions. (You have a similar structure in languages from the APL family, where operators are all unary or binary, and traversal is specified by adverbs to the operators.)

This does of course not apply if you use a deliberately restricted language that has just one type of collection traversal, in that case you are better off with varargs, as n*1 < n+1.

lyudmil• 15 years ago

I try to be very pedantic about having the most terse, yet readable code possible in my application. This probably means that if, for now, I only need to know how to add two vectors, I will only include a function that adds two vectors and no more. I do realize there's hidden structure there waiting to be exposed, but programming is largely about elaborating the useful structures. If the author's examples had been in Java instead of Clojure, a lot of us would've been complaining that he's over-designing.

I do think the author has a point, but to me the logical complexity progression for a piece of code is:

1. Make it work in the "1" case.

2. If necessary, make it work in the "2" case.

3. If necessary, make it work in the "n" case (n variable).

I find this particularly useful for larger decisions: I can make this work right now if I add this tiny little wart? No problem. Another thing came up and a similar wart is required? Ok, I guess. A third? Time to find the structure that my code is telling me about and refactor to expose it.

humbledrone3• 15 years ago

The final (varargs) add-vectors function in the OP is clearly (at least to me) more terse and readable than the original (2 arg) one. I've only written a couple thousand lines of Clojure to date, but code like that already just looks right. The varargs version has less possibilities for errors; the 2 arg version allows for several permutations of first/second to be written incorrectly, or the wrong va/vb used.

cpeterso• 15 years ago

In my code, I limit my hardcoded constants to 0, 1, -1, and '\0'. Any other numbers, I give it a named constant, even if it's just defined in a local function scope.

MostAwesomeDude2• 15 years ago

Since we've already descended into anecdotal territory, lemme drop an anecdote. I had a dot-product function which turned out to be a hotspot in some numerical code. The original function looked something like:

    def dot(v1, v2):
        return sum(i * j for i, j in zip(v1, v2))

Very functional, very pretty, very straightforward. But it was slow, both in CPython and PyPy. A bit of bytecode analysis and profiling later, we realized that unrolling would be faster by several orders of magnitude. So I checked this in:

    def dot2(u, v):
        return u[0] * v[0] + u[1] * v[1]
    def dot3(u, v):
        return u[0] * v[0] + u[1] * v[1] + u[2] * v[2]

And the bottleneck went away. From the Zen of Python: "Practicality beats purity."

ZitchDog• 15 years ago

Any reason you didn't just use NumPy?

http://docs.scipy.org/doc/numpy/reference/generated/numpy.do...

MostAwesomeDude2• 15 years ago

We considered it, since Numpy was already a dependency, but it was even slower than the current code.

puredanger2• 15 years ago

If you spend any time in the Clojure community, I think you'll come to an appreciation that practicality and performance are key principles of its design. The goal is to obtain both high levels of abstraction and performance.

NickPollard2• 15 years ago

But performance means different things to different people.

If you're writing code where dot products are not something you do a lot of, then keeping a clean, idiomatic functional implementation is probably fine.

If you're writing code where dot products happen a lot, such as on real-time three-dimensional games (which happens to by my oeuvre), then the performance increase of having specific, unrolled implementations is huge.

daemin• 15 years ago

I think your original function was pure in the mathematical sense, but since you were operating on a data structure that is specifically 2 or 3 elements long, then you don't need to be that generic.

Just think of a vector of 3 elements as being one entity, and therefore it still obeys the 0, 1, Inf rule mentioned elsewhere in the comments.

kevinlawler• 15 years ago

K handles the general case:

    1 1 1 + 2 2 2
  3 3 3
  
    (1 1 1; 2 2 2; 3 3 3) + (4 4 4; 5 5 5; 6 6 6)
  (5 5 5
   7 7 7
   9 9 9)

hazexp• 15 years ago

As does R :)

    > c(1, 1, 1) + c(2, 2, 2)
    [1] 3 3 3
    > matrix(c(1, 1, 1, 2, 2, 2, 3, 3, 3),
    +        ncol = 3, byrow = TRUE) +
    + matrix(c(4, 4, 4, 5, 5, 5, 6, 6, 6),
    +        ncol = 3, byrow = TRUE)
         [,1] [,2] [,3]
    [1,]    5    5    5
    [2,]    7    7    7
    [3,]    9    9    9

Not quite so concise and elegant but it's pretty straightforward stuff for most R users.

2 is a code smell

Related Stories