6

I've often seen it argued that heavily nested function calls should not be used because they are unreadable. However, using temporary variables instead creates a lot of unnecessary verbosity and forces the reader to mentally link each temporary variable to what it represents. When looking at the way Lisp code is typically formatted, it occurred to me that nested function calls can actually be made quite readable if you format them to reflect the nesting. For example:

// Totally unreadable:
auto linesIter = filter!"a.length > 0"(map!strip(File(filename).byLine())))


// Perfectly readable.  The only difference is formatting.
auto linesIter = filter!"a.length > 0"(
    map!strip(
         File(filename).byLine()
    )
);

// Readable, but unnecessarily verbose:
auto rawLines = File(filename).byLine();
auto stripped = map!strip(rawLines);
auto filtered = filter!"a.length > 0"(stripped);

Writing something like the first example in nested function form is, IMHO, equivalent to doing the following in more procedural-style code:

for(i = 0; i < 10; i++) { for(j = 0; j < 10; j++) { if(x < 2) { z++; } else { y++; }}}

In both cases the real problem is poor formatting, not excessive nesting. How would you rate the readability/comprehensibility of the well-formatted nested function version vs. the temp variable version? Do you think heavy function call nesting is bad style even if it's formatted for maximum readability? If so, why?

Mankarse
  • 39,818
  • 11
  • 97
  • 141
dsimcha
  • 67,514
  • 53
  • 213
  • 334
  • Just out of curiosity: what language is this? I don't know D, but the `auto` keyword and the lambdas encoded as strings look like it. And if it is, is it D1 or D2 (or is it legal in both)? – Jörg W Mittag Feb 02 '10 at 19:09
  • @Jorg: Yes, it's D2. The filter and map constructs are from std.algorithm. – dsimcha Feb 02 '10 at 19:42

5 Answers5

3

I see nothing wrong with breaking large nested function calls in the way you have using formatting. It's readable, which was the intention.

With temporary variables, I try to avoid them. And the only reason for this is quite simply, I am utterly incapable of giving them good names. I try when they are absolutely necessary, and end up wasting 5 - 10 minutes thinking 'what would be the good name for blah blah that does cor blimey'? And almost always end up giving up and using buffer if it's an array, or temp if it's a scalar, etc.

That said, with your nested structure, the functions represent verbs, and not the products of the verb's action, which is what you'd get with temporary variables. With temporary variables, you do get a chance to give that product a name. But if it's a thing that is better described by the verb and direct object themselves (verb(directObject)) then that's the route you should take.

Anne Quinn
  • 12,609
  • 8
  • 54
  • 101
2

You say "using temporary variables instead creates a lot of unnecessary verbosity and forces the reader to mentally link each temporary variable to what it represents" -- but IMO that's just another way of saying that you've broken up the thing into steps that the reader can understand one at a time -- in other words, you've made it more readable.

I'm quite happy to add an extra variable to break a long line up into seperate steps (your example 3), but the key thing for me is whether you can break the thing up cleanly into genuine steps. One good indicator is whether you can find a good variable name; if you can't, maybe it's not a genuine seperate step that needs splitting out.

There's nothing much wrong with your example 2, but much longer than that and I would certainly break it up. You'll thank yourself when it comes to debugging...

Shadowfirebird
  • 757
  • 3
  • 13
1

IMHO, objects and names should be carefully chosen so that things are readable when you do the function-style coding of slamming everything onto one line like in your first example. If it is totally unreadable, somebody(s) chose bad names. Your goal is that if something looks off, often times it means there is a bug.

In the real world when you have to deal with those badly-chosen names, there isn't anything wrong with creating temporary constants (or even routines) with proper names to clean things up a bit. It beats a comment, because it compiles and is easier to modify.

T.E.D.
  • 44,016
  • 10
  • 73
  • 134
0

I personally think that if there is no performance penalties readability is more important. Although i do not know anything about the Lisp syntax, looks like your examples makes the same number of calls? If thats the case, do it readable way. But if you are calling a heavy function (like opening a file and reading again and again) more times in a loop, you should avoid it. I hope this answer is useful.

Deniz Acay
  • 1,609
  • 1
  • 13
  • 24
0

I prefer not to use too many variables b/c they are often the source of bugs. If I can express something as a single expression I prefer it that way. I would probably go with your second example.

Markus Johnsson
  • 3,949
  • 23
  • 30
  • I'd say that complex expression are more likely to contain bugs, especially if somebody else (or yourself after 6 months) carries out maintenance. – beny23 Feb 04 '10 at 23:38
  • I based my statement on what I've learned from functional programming. In functional programming the philosophy is that maintaining state and mutable variables is the source of bugs; expressions and functions are preferred because they are declarative. I.e. express what we want done not by a series of steps but as a single expression. – Markus Johnsson Feb 05 '10 at 08:36
  • I guess it's personal preference. I would say that if something looks simpler and readable, then it's more maintainable and a lesser source of bugs. Sometimes that's a single expression, sometimes it's a few lines of code with some variables with meaningful names. – beny23 Feb 16 '10 at 22:52