Is Functional Abstraction Too Clever?

I received a rather interesting comment on a recent Stack Overflow answer:

This code seems too clever by half. Is it art? – PeterAllenWebb

The code in question was a functional solution to an algorithm described approximately as follows:

Draw n−1 numbers at random, in the range 1 to m−1. Add 0 and m to the list and order these numbers. The difference between each two consecutive numbers gives you a return value.

Which I solved like this, with n = slots and m = max:

static int[] GetSlots(int slots, int max)
{
    return new Random().Values(1, max)
                       .Take(slots - 1)
                       .Append(0, max)
                       .OrderBy(i => i)
                       .Pairwise((x, y) => y - x)
                       .ToArray();
}

Using a few extension methods:

  • Values() returns an infinite sequence of random values within the specified range.
  • Append() takes a params array and appends its arguments to the original sequence.
  • Pairwise() generates a sequence from calculations on pairs of consecutive elements in the original sequence.

I can see how one would think the code is clever; however, I’m not sure what would qualify it as too clever. Every method call has a well-defined purpose and maps directly to part of the original algorithm:

  1. From random numbers in the range 1 to m−1…
  2. …draw n−1.
  3. Add 0 and m to the list…
  4. …and order these numbers.
  5. The difference between each two consecutive numbers…
  6. …gives you a return value [in the array].

As far as I’m concerned, a solution couldn’t get much clearer than
this, but that’s easy enough for me to say—what do you think? Is there
a better way to express the algorithm? Would an imperative solution
with shared state be more readable? How about maintainable?

For example, one could add the requirement that the random numbers
not be repeated so that the difference between adjacent numbers is
always nonzero. Updating the functional solution is as simple as adding
a Distinct() call:

    return new Random().Values(1, max)
.Distinct()
                       .Take(slots - 1)
                       ...

To me, this is the value proposition of functional programming. By
expressing the algorithm in terms of common operations, we’re able to
spend more time thinking about the problem than the details of the
solution. A similar change in an imperative implementation would almost
certainly have been more involved and prone to error.

For completeness, here are the implementations of the extension methods used:

public static IEnumerable<int> Values(this Random random, int minValue, int maxValue)
{
    while (true)
        yield return random.Next(minValue, maxValue);
}

public static IEnumerable<TResult> Pairwise<TSource, TResult>(
this IEnumerable<TSource> source,
Func<TSource, TSource, TResult> resultSelector)
{
    TSource previous = default(TSource);

    using (var it = source.GetEnumerator())
    {
        if (it.MoveNext())
            previous = it.Current;

        while (it.MoveNext())
            yield return resultSelector(previous, previous = it.Current);
    }
}

public static IEnumerable<T> Append<T>(this IEnumerable<T> source, params T[] args)
{
    return source.Concat(args);
}

This also reminds me that Jimmy posted a similar Append() method as part of his latest post on missing LINQ operators. I used to use a version similar to his, but have found the params version to be more flexible (and easier to implement). Its Prepend() counterpart is similarly trivial:

public static IEnumerable<T> Prepend<T>(this IEnumerable<T> source, params T[] args)
{
    return args.Concat(source);
}

Related Articles:

About Keith Dahlby

I'm a .NET developer, Git enthusiast and language geek from Cedar Rapids, IA. I work as a software guru at J&P Cycles and studied Human-Computer Interaction at Iowa State University.
This entry was posted in Functional Programming. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://paulbatum.com Paul Batum

    I liked your solution and upvoted it. I tried to find an alternative implementation of pairwise that relied on the existing IEnumerable combinators rather than your imperative approach of manually using the enumerators but I had no luck :(

  • Jason Meckley

    I think some find this “too clevel” because they are not used to intention revealing interfaces/methods. the VS IDE wizards produce a lot of code like Button1 and Button1_Click. this doesn’t tell us anything about the purpose of the control or what it does. We are forced to look at the properties of button 1 to find out it’s value is Save and that clicking it will update a record in the database.

    In this instance you have intention revealing extension methods. but most .net devs must see the internals of the method to understand what’s happening. In other words they cannot understand the “what”, without knowing the “how”.

  • http://adamjwolf.com Adam Wolf

    Too clever, No. This is a great example of how functional programming dissects problems and solves them. I can’t imagine people who actually know how LINQ, Yield and extension methods would think this is too clever.

    The level of abstraction in the GetSlots method is perfect. I guess some people need to have everything in front of them to feel comfortable about the code but are willing to use the BCL without browsing the source.

    Does F# have a chance in corporate america?

  • http://www.codemonkeyism.com Stephan Schmidt

    Nice post. I’m not sure if all this functional programming stuff doesn’t just boild down – in these examples on list processing – to a pipeline. I’ve solved many problems of this kind with an OO pipepline of Mappers and Filters.

    Oh and I like functional programming.
    Cheers
    Stephan
    http://www.codemonkeyism.com

  • Jeremy Gray

    This code is not too clever. In fact, it would be difficult to create a more direct representation of the stated requirements! The meaning of Pairwise might be the only part someone would find unfamiliar and even that part takes only a moment to understand.

    Code one step too clever obscures intent (and bugs) as much as the old-school procedural style code that was accepted over your version. The goal is to find the sweet spot, which I think you’ve done nicely.

    Had that accepted implementation contained a bug, it would almost surely be lost in the noise and harder to spot than the small bug I very quickly spotted in yours. :D

    The fact that I could spot it so quickly is, IMHO, a very good thing. Good code strives to make intent clear and bugs obvious.

  • Jeremy Gray

    re: there being a bug – unless I’m having a Complete Saturday Brainfart about the behaviour of the Values extension method, that is. ;)

  • http://www.lostechies.com/members/dahlbyk/default.aspx Keith Dahlby

    @Paul ~ There really isn’t a clean way to represent Pairwise with the standard operators – the F# implementation is imperative as well.

    @Adam ~ I don’t think corporate environments care about the language used, per se. They care about support and availability of developers. Until MS can prove that F# is ready for the big time (they’re trying) and there are more than a token number of developers that really know F#, I don’t see that happening. Eventually we’ll get there, but in the meantime I would advocate all developers learning how to apply functional concepts in their current language of choice to ease the transition if/when that day comes.

    @Jeremy ~ I don’t believe in bugs, only undocumented features. ;) My intent with Values is to provide an infinite sequence generated with Next(min,max), with terminating the sequence delegated to Take(While). I think it needs a better name than Values, but I can’t think of one that I like.

    @All ~ I figured the perceived cleverness was due to its non-imperativeness, but sometimes it’s hard to be objective about solutions that I rather like. Thanks for the feedback!

  • http://blog.codinglight.com/ David Morton

    Check the StackOverflow link. I’ve posted another possible solution that seems to work for the purposes, and still seems to be relatively fast. It’s certainly inspired by the code on this post, however, but I’ve chosen instead to use a dictionary instead of creating a whole pairwise method, though Pairwise isn’t a bad idea.

  • Jeremy Gray

    @Keith – For what it’s worth, I double-checked Random.Next’s behavior just now and I was in fact having a brain fart earlier today. Your impl is both clear in intent and clear of bugs. :)

  • http://geekswithblogs.net/chrisfalter Chris Falter

    Looks like there’s a bug in calling Append(0, max)? Based on the implementation of Append, it likes like it concatenates 0 and max as the last 2 members of the list. That’s what IEnumerable(IEnumerable first, IEnumerable second) does–in your case, “0, max” being the IEnumerable second.

    Instead, I think you would have to do the following:

    .Prepend(0)
    .Append(max)
    // etc.

    Other than that minor bug, I love the idea and the code. Excellent post!

  • http://www.lostechies.com/members/dahlbyk/default.aspx Keith Dahlby

    @Chris ~

    You’re right except that after Append() we’re sorting the full sequence so the 0 will end up first anyway – a slightly more optimized (but maybe less readable?) version would sort the random values first and then prepend/append as you said:

    .Take(slots – 1)
    .OrderBy(i => i)
    .Prepend(0).Append(max)

  • http://blog.casualdev.net Alexander Abramov

    Nice post and comments, functional style needs to be brought to broader audience. It is is just a tool with its own uses, and a very expressive tool at that.