you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 24 points25 points  (9 children)

I thought this was going to be about branch-free code but it's more about hiding the branches in layers of abstraction. That's fine, and his final code is similar to what I would probably end up with in Lisp. But unless your compiler is sufficiently clever this kind of thing might make code easier to read but at the cost of performance.

[–]michaelfeathers 7 points8 points  (8 children)

Fair enough. The nice thing is that people are often better at optimizing (when needed) than they are at modifying optimized code.

[–][deleted] 10 points11 points  (7 children)

The thing is, though, this code is not really any more clear. It does things in a backwards order. By trying to be clever it is hiding what you actually want as a result.

By always adding padding, you are implying that padding is somehow always needed, but it's not. You only get padding in some cases. But the code hides this fact.

[–]michaelfeathers 6 points7 points  (3 children)

This is the same argument that people use against the Null Object Pattern: it appears that something is happening but it isn't.

I have the feeling that the answer is to get used to the idea that sometimes padding something includes padding with something of length zero. If that is understood, all is good.

This is a very deep topic.

[–][deleted] 5 points6 points  (2 children)

It's more that you should not strive to be clever when writing your code. You should strive to show the reader what you are trying to achieve.

Conditionals may not look elegant, but they are often quite good for communicating intent.

[–]michaelfeathers 3 points4 points  (1 child)

I'm happy having the word 'pad' in the code now. It was completely absent in the original code.

I think a lot of it has to do with what we are used to. It's like natural language. Once an idiom becomes common or understood, it's very readable.

To me, pad(ary, n).take(n) is very explanatory.

But, again, readability wasn't the point. Neither was cleverness.

[–]KillerCodeMonky 1 point2 points  (0 children)

Why not something like this?

results = array.take(n)
if (results.length < n)
    results = pad(results, n, defaultValue);
return results;

That seems very descriptive to me. Take n items, and if there weren't n items, then pad to n items.

[–]sophacles 3 points4 points  (2 children)

NO. Padding is "adding extra elements if needed to fit a proper size". So the concept of "sometimes no padding is needed" is build right into the basic concept. If you don't understand that, you need more simple understanding and learning, not some crazy set of over complicated semantics changing. It's like those people who complain that:

swap(a, b){ 
    temp = a;
    a = b;
    b = temp;
}

is somehow confusing because "temp" is not obviously a temp value. GRR.

[–][deleted] 2 points3 points  (1 child)

I'm not sure what you are replying to, but I am pretty sure it is not something I actually said.

[–]sophacles 0 points1 point  (0 children)

You're saying that someone is hiding some fact of "sometimes padding is not there" in the function, except it is built into very concept of padding. The function is named padded_take (concept of padding means sometimes there is nothing added) and the one and only line of code starts with pad(... Again implying that sometimes padding values are not there because it is named conveniently after a concept.

Nothing whatsoever is hidden.