Functional code != Good code

There’s a dangerous trap to fall into: The belief that functional code is automatically good code. Hopefully not too many people would come out and actually claim this, but it seems to be an unstated common belief. It’s also utter bollocks.

Functional programming gives you tools for writing good code. Good functional code can be very good (good imperative code can be very good too! But that’s not the point of this post). Bad functional code can be just as bad as its imperative cousin.

Now, what post about functional programming would be complete without some trivial one liners? Let’s start by summing a collection of integer elements.

Imperatively:

   var x = 0;
   for (j <- myCollection) x += j;

Functionally:

  val x = myCollection.foldLeft(0)(_+_);

The functional code is a little shorter. You’ve avoided making the sum variable mutable, which is nice. It introduces one fewer local variable which helps with reading the code (but it wouldn’t if Scala didn’t have the cut notation for anonymous functions, you’d have had to write (j, i) => j + i, which introduces *more* variables). Is the fact that you’ve used a higher order function an improvement? No, not really. The loop is a higher order function too. We could have written it:

   var x = 0;
   myCollection.foreach(x += _);

and it would have been exactly the same thing (literally. The Scala compiler emits something nearly identical to the second version when given the first version). So the only difference here is the lack of mutability. Both examples are trivial, so there isn’t a whole load of difference. But most code is just a sequence of trivialities, so considering how they’re written is valuable.

One advantage of the fold here is that you can use it as an expression without binding it to a variable. The loop doesn’t let you do that. But is that really a good thing for you to be doing?

Thing is, both of these examples are the wrong way to do it in the middle of other code. The right way to do it?

  val x = sum(myCollection);

(Unfortunately there is no sum method in the Scala standard library. A minor wart. But there’s one in e.g. scalax, or your personal utility classes, or wherever you care to look really).

Inline calculations like this should be factored out into simple methods. This is a no brainer, everyone knows it’s a good idea. And the fact that you’ve written your code in a single expression with a fold doesn’t exempt you from that.

Let’s make our trivial example very slightly less trivial. Instead of a collection of integers, I have a collection of collections. I want to find the total length.

Imperative version:

  var x = 0;
  for (c <- myCollection) x += c.size;

Now we’ll see how the functional code really shines in this example:

Functional version:

  val x = myCollection.foldLeft(0)((y, c) => y + c.size)

See how much better abstracted that is than the imperative version?

Um, no. Me neither.

When you’re doing folding like this if you don’t have an already defined function to be reusing (which doesn’t have to be a named constant, it could be the result of some other computation. It probably shouldn’t be a lambda), this really isn’t any better than imperative version. At least to my eyes I find it a bit worse, but that’s largely a matter of opinion. It’s sure as hell not better.

So how *should* you be writing it? Am I now going to say you should define a method sumLengthsOfCollections?

Fuck no.

You should solve it, as with so many other things, by reducing it to a problem you’ve already solved.

   val x = sum(myCollection.map(_.size));

Here you actually *have* reused logic properly, and the code is at least somewhat better. And if you wanted to later change the implementations of sum (Think this is a ludicrous comment? Really? What if this were acting on BigInts rather than integers. Still think it is?).

In these trivial examples it doesn’t make much of a difference, but as the number of variations and steps in the pipeline increases this style of code really starts to make a difference. Earlier today I had to deal with a method definition that involved a really large number of folds, most of them doing trivial things. I was not amused. The fact that you can fold, doesn’t mean that you should, and the fact that you’ve been given higher order functions isn’t a license to use the wrong ones and assume your code will be good anyway.

19 thoughts on “Functional code != Good code

  1. Daniel Spiewak

    I’ve seen a fair bit of code which over-uses functions like fold and flatMap. Heck, I’ve even *written* code like that! I think the reason it exists is a lot of people using functional programming in any language (including Scala) are those who learned its precepts under the cruel lash of a more strict language. Think about it, if you learned how to code in Haskell or even Lisp and then tried to learn Scala, you would end up producing some hideously functional code. Now, this isn’t as bad as it could be in languages like Java using its sorry-excuse-for-a-lambda, but it is still a problem (if nothing else, because non-trivial Scala is much more verbose than the equivalent Haskell or ML, partially due to the differences in type inference algorithms).

    I think that your point is spot on: don’t marry yourself to a particular methodology just because it solves *some* problems well (or even worse, just because it’s “hip”). Even Scala’s standard library adheres to this principle, implementing functional constructs (like everyone’s favorite persistent collection: List) using imperative algorithms for efficiency.

    While I think that it is *rare* that a trivial computation such as the ones you illustrated can be cleaner imperatively than in the functional style, there do exist many examples of a non-trivial nature for which this holds. The beauty of Scala is that it doesn’t restrict you to one or the other, letting you mix and match, getting the best of both worlds. I suppose that sounds like PR, but it really does solve this problem of divergent methodologies quite nicely.

    Reply
  2. Collin Cusce

    I’ve talked quite a bit on what makes “good” code on my site, as well. Functional programming does make code extremely succinct, and yes there are “better” ways to do most code. The question this leaves me asking still is what is good code? Efficiency? Code Reuse? Readability?

    I’ve found that if it “works” it must fit the basic definition of “good”. While there is better ways to do most anything these days, good is often good enough.

    I enjoyed your article, though. I think you’re correct in what you’re essentially saying… just because it’s FP doesn’t mean there’s not more than one way to do it.

    Reply
  3. Tony Garnock-Jones

    There’s been a discussion about #sum on the squeak-dev list recently. I don’t like unary #sum, because it doesn’t work with empty collections in the absence of either a powerful static type system or a sum protocol that has a “universal zero” (to coin a term). In dynamically-typed languages, #sum should instead be #sumFrom:, which is much more directly analogous to your fold. (Note how the appropriate zero is mentioned in the fold.)

    myCollection.sumFrom(0);

    Can Scala guess an appropriate zero, based on the type of the elements of the collection, for use with .sum()?

    Reply
    1. Karl Bielefeldt

      This is an old thread, and I don’t know what capabilities Scala had 6 years ago, but now it’s quite capable of choosing an appropriate zero, without the need to guess. All collections now have a built in sum method, which takes an implicit Numeric[T], which has a method that returns a zero specific to that type, as well as defining an addition operation. This lets sum() work seamlessly with all the built-in numeric types, as well as giving you the ability to define a zero for your own numeric types.

      In Haskell, defining a zero for a sum function is just as easy with the Num type class, which I know was around way before 6 years ago. This is hardly an intractable problem.

      Reply
  4. Muharem Hrnjadovic

    Sigh, another discussion about good/bad code without prior definition what is meant by these attributes.

    What is “bad” in your eyes may be “good” in mine and vice versa. It usually boils down to your cognitive setup (previous experience, preferences, priorities etc.).

    Reply
  5. david Post author

    Daniel:

    Certainly for trivial computations I find that the functional style works better. For non-trivial ones it’s often better, but occasionally you have to vastly reorder your thinking in order to get results that are only somewhat better (or even worse) than the imperative version.

    Tony:

    Unfortunately, Scala’s numeric hierarchy sucks profoundly, so there’s not a good way to abstract between different types of numbers. Even getting sumFrom to work properly is a bit of a nuisance. In principle this is just an implementation detail though, and it should be possible to write a sum method that does the right thing based on type (It’s even mostly possible to retrofit this onto the hierarchy without changing the library implementation or language, but no one has done so).

    Collin, Muharem:

    There’s a time and a place for stating your axioms and elaborating on the meaning of a term. I didn’t feel this was it.

    For these purposes I’m talking mostly about code reuse and code complexity relative to the task solved.

    Reply
  6. Erik Engbrecht

    Ok, now how would you write the sum method: imperative or functional?

    I don’t think the issues your addressing is really an imperative versus functional. It’s an issue of “how small of common expression is too small to factor into another method/function?”

    I think you’re saying: “If the meaning of the expression is unclear and the meaning of the name will be clear, make it a named method/function.”

    That makes sense to me, until I end up with 20,000 methods with names longer than the expressions they contain…

    Reply
  7. david Post author

    I’m not actually saying anything about imperative vs. functional. I’m saying “If you’re going to write functional code, do it right”. Exactly the same point could be made about imperative code, but it seems a little more obvious in that context because people are more familiar with it.

    RE the implementation of sum, I don’t really care how the sum method is implemented to be honest. If Iterable had a reduce method (as opposed to reduceLeft and reduceRight) I might advocate sum(x : Collection[Int]) = if (x.isEmpty) 0 else x.reduce(_+_) to prevent from introducing artificial order dependencies. But that’s neither here nor there.

    As far as factoring into small methods go: Obviously you have to find a middle ground. There are various rules of thumb as to when to do this and when not. I’m certainly not advocating def plusOne(x : Int) = x + 1. In fact, I’m not really advocating anything at all. This post is more of a cautionary tale than an instruction set to follow.

    Reply
  8. Evan

    David,

    I don’t believe that your example demonstrates what you’re attempting to demonstrate. Functional programing IMHO is more than replacing for loops with map/fold/recursion etc. The key benefit is referential transparency. Or avoiding *visible* side effects. Both the functional and the non-functional examples could be wrapped up in a function and both functions would be referentially transparent, and therefore would both be employing the functional programming paradigm. In fact if you were to look under the covers of the foldLeft function you may find out that it mutates some variable somewhere. Likewise if you look at the code that is generated by ghc (a haskell compiler) you’ll see that some of the code it generates is also imperative. That doesn’t make haskell an imperative language, because whether or not there is mutation happening under the covers is irrelevant in haskell all functions are referentially transparent and the haskell programmer can pretend that no mutation is taking place ever. It’s probably possible to come up with some examples where an imperative approach is better than a functional approach, but to really show that you’re going to have to use more than a single line of code. You’ll have to have multiple functions, where at least one of them modifies some global, or object variable, and that causes at least one function to return different results given the same input. I think demonstration can be done, but it will take more work.

    Reply
  9. Pingback: David R. MacIver » Blog Archive » A follow up to yesterday’s article

  10. Daniel Spiewak

    WRT the implementation of sum, I would *like* to do it this way (doesn’t compile):

    type T = { def +[A <% T](o: A): T }
    def sum(nums: Iterable[T]) = nums.reduceLeft[T](_+_)

    Scala doesn’t support self-referencing types (classes are fine, types are not). I’m sure there’s some deep theoretical reason why this is, but it does get a little annoying for situations like this one.

    Reply
  11. david Post author

    Daniel:

    I wouldn’t like to do it that way because the structural types implementation sucks. :-)

    The way I’d like this to work is with a decent numeric hierarchy in Scala. Unfortunately we don’t have one.

    Reply
  12. Pingback: mein Blog » Blog Archive » funktional kontra imperativ

  13. Pingback: Recent Faves Tagged With "integer" : MyNetFaves

  14. Stephan Schmidt

    “[…] and therefore would both be employing the functional programming paradigm.”

    Side effect free code is a good practice to follow and has nothing to do with functional programming – just because it is a side effect to functional programming.

    I find it irritating that the practice of side effect free code his hijacked by functional programmers and claimed to be “functional”.

    It’s like anyone using cheese on top of something is employing the “Pizza paradigm”.

    Peace
    -stephan

    Reply
  15. Pingback: Functional code != Good code - Tech cents

  16. Pingback: Best of drmaciver.com | David R. MacIver

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>