Hacker News Reader

Comment by dkarl | Hacker News Reader

> The best measure for the quality of the codebase is whether at a glance you can understand what's going on.

This is _so_ relative to the background of the people doing the glancing. These days [1, 2, 3, 4].map(x => x + 12).filter(x => x % 2 == 0) is obvious at first glance. Twenty years ago most people would have begged you to rewrite it with for loops.

Right now I am in a hell of trying to figure out if the Scala codebase I'm working on is terrible or if I am just not fluent enough yet with FP and cats and related libraries. There are some points of style I'm confident are poor choices, but when it comes to other aspects that seem horribly convoluted to me... I'm still not sure if the code is written for somebody with more experience in the style, or if it's a poorly executed example of the style.

View on HN

I agree with you that is obvious at first glance now a days but I wouldn't want to see it written like that in Python at least...

I would prefer to have small functions named after what it is accomplishing and then have 2 function calls..

something like

  list_of_grades = [1 , 2, 3 4]
  adjusted_list_of_grades = 
  apply_end_of_semester_grade_adjustment(list_of_grades)
  odd_grades = remove_even_grades(adjusted_list_of_grades)

Of course that this example is very silly but understanding why a transformation is happening when you're looking an old code base that you don't have the context is easier w/ a function and docstring that explains it than a map or filter

(IMO)

I think also we have learned why map/reduce or functional recursion is better than for loops - because the state of the system is explicitly contained; with a for loop you could literally mutate anything in scope, so the cognitive burden to understanding the process is potentially unbounded; with map, your state between iterations is nothing, and with reduce, it's strictly what you can stash in your accumulator.

I was laughing at myself the other week because I was having so much trouble writing a for loop because I had gotten used to the explicitness of the functional iteration operatiors

> with a for loop you could literally mutate anything in scope

That depends on the language. In Elixir for instance, you can't mutate anything. Elixir does have 'for comprehensions', and there are things that you can express very clearly in what is effectively a for loop that would be much harder to read in a chain of iterators.

That's not strictly true. You could use the process dictionary to keep hidden mutable state in the for loop. Don't do this.

Same here. I maintain around 30 small-to-medium size Ruby projects and in all that code I can only remember writing a single for loop. On the plus side, I am highly confident that code works right since I spent so much time thinking about it!

Mutation is the scapegoat here. It's all about expressing intent.

Eg: I would take a count_if(condition) function over filtering and then taking the length.

I understand, but that's IMO far too wordy. You will get better at reading things functionally and you'll start to move towards the original one-liner style, but some people take it too far.

I actually did last night what you prefer, pulled a small lambda out of a map and named it.

The problem with this is that you've replaced the code with comments about the code - that's what long method names are, comments. And like comments, they stagnate while the code they reference evolves.

Someone else reading this code in a few years time will need to look into the definitions of each of those method calls in order to understand the code, because names can't be trusted. All the more so with instance methods, as they may access arbitrary instance state to do their work, so that non-local (to the calling point) state may influence meaning.

Single use small methods need to justify their existence to avoid being inlined; they need a smidgen more purpose than a comment, or they hurt readability long term more than they help.

I understand where you are coming from but I don’t agree.

Yes in a low quality codebase with focus on producing changes quickly and not maintainability it is true that the code will change but comments/names stagnate. What you are referring to is reinforcing a problem a cultural and organizational issue, which left unkept will make progress stagnate in the long run.

But in a high quality codebase armed with proper peer-reviews this divergence of name and implementation won’t be tolerated and if such divergence exists it should be considered a bug not the expected state of things, such a defect should be resolved when found instead of making it the norm that you can’t trust the code base.

What if we couldn’t trust that parts in our cars and heavy machinery does what they say, I wouldn’t want to tear down the engine every time I’m about to use a car just to make sure there’s actually an engine inside and it’s not a fridge compressor due to implementation diverging over generations. This is of course an extreme example but what I’m saying is that we should allow our code to decline into such a state to begin with.

Are the downvoters perl programmers?

> high quality codebase armed with proper peer-reviews

This is a No True Scotsman argument. Of course if you assume a process which prevents problems, you won't get problems.

I don't believe it though. People make compromises in the face of conflicting demands; technical debt is taken on in order to get features to market sooner. Developers churn; new developers are hired who have less context and take shortcuts, and other newer developers review and approve their code. In large systems, developers may have been working for years but still be unfamiliar with different corners of the codebase; newness is path dependent.

> What if we couldn’t trust that parts in our cars and heavy machinery does what they say

This analogy doesn't really fly. Software isn't subject to physical constraints on local action.

Functions are abstractions. There's a handy rule of thumb about abstractions: don't create an abstraction until you have three different uses. Now I think functions are fairly lightweight abstractions and are generally malleable, so I wouldn't really apply it. But the principle behind the rule is still sound. Multiple users keep an abstraction honest. They stop it growing hairs and warts specific to a single user, which bleed hidden dependencies across the abstraction boundary.

It's all context dependent of course but in this example the original example is far more readable. Sometimes more abstractions make it harder to understand how everything fits together.

If you really want to document it more wrap the map/filter inside a single function, assign it to an aptly named variable or add a comment above.

You also introduce a refactoring problem when you split it out in functions as you now need to consider that it may be called in other places too.

The Pythonic way though is to use list comprehension and generator expressions over map/filter.

To me as well, good readability means not having to read too much.

Here is a code example from the front page of the official Python website [python.org]:

  >>> numbers = [2, 4, 6, 8]
  >>> product = 1
  >>> for number in numbers:
  ...    product = product * number

Compared to the Ruby equivalent,

  product = [2, 4, 6, 8].inject(:*)

this feels like unnecessary bloat that makes it harder to understand what is going on in the larger picture, i.e. the unit of code this is a part of.

I think this is reading too much into the toy example, but I want to mention a drawback of your rewrite: in the original, map and filter made it clear that all logic was happening element-wise. By creating functions that take the entire list as input, this has been obscured.

I think I'm in between somewhere. I do love named intermediate results as documentation. I love that functional code composes so well that you don't have to give names to every value... but I hate when people abuse it to eliminate all names.

Oftentimes, if I have a one or two line function that is only used in one place, I prefer to have it inline and use a named intermediate value to document its meaning.

  grades_with_end_of_semester_adjustment = grades.map(x => 
    code code code
      code code code code code
  )

  odd_adjusted_grades = grades_with_end_of_semester_adjustment.filter(x => 
    x % 2 != 0
  )

> This is _so_ relative to the background of the people doing the glancing.

This is very true. I am running a project at work using Elixir, in a software group that writes most of its code in C. I am curious to see how our style evolves as we become more intimately familiar with functional tools like chained iterators.

I might find it obvious but would probably still like a rewrite, also maybe trusting it depends on how much you trust the person writing it.

But definitely the point stands, the readability of code is often a function of experience and ability on both the reader and writer's sides.

A recent bandwagon to jump on is that you should never use reduce() for anything whatsoever, because it's always easier to read if you rewrite it.

I'm not sure about it, but I have certainly seen some overengineered code using reduce().

'reduce' (AKA 'fold') is rather fundamental to lists/sequences: it's their 'elimination form', which means anything involving lists can be written using 'reduce'. Functions like 'map', 'filter', etc. are essentially common patterns which can be implemented using 'reduce'.

I understand the aversion to 'reduce', since it can get quite messy, but I still prefer it to e.g. WHILE loops (note that the 'for' keyword in most languages actually implements a WHILE loop). I think a more general rule is that custom abstractions can sometimes be useful, so we shouldn't try to write everything in terms of language builtins (these days 'map', 'filter' and 'reduce' are often built-in, but we can still make our own abstractions on top if appropriate).

As a comparison, the elimination form for booleans is 'if/then/else': we could write all of our branching in terms of if/then/else, but there are common patterns that can be expressed using abstractions like boolean algebra (AND/OR/NOT/etc.).

Reduce and fold capture the structure, but there is a readability problem with them, which is that the semantics of the accumulation value are often complex and not obvious. It's nice to give the accumulator a descriptive name, but the more complicated the value, the more likely a programmer is to give up and call it something unhelpful like "x" or "accum" instead of "mapOfAccountIdAndSectionIdToCountAndSumAndCountOfTransactionsExceedingLimit".

To make sure code like that is readable, programmers have to declare types even when they're optional, use descriptive names, and use comments when these methods don't suffice. Or in Scala, even declare case classes that are only used in a single complicated expression, which sounds extravagant, but when I've seen it, it turned code that might have taken ten minutes to decipher into code I could cruise right through. Unfortunately, in my experience, this is rare. Often my first step in figuring out someone else's reduce or fold is to guess how I would have done it and then see if their code implements my guess, which is an assembly language level of readability.

I completely agree, using 'reduce' can make code hard to figure out. I'm very guilty of this myself, e.g. I've written a bunch of hacky mess which roughly follows this template:

    snd (reduce (FOO, BAR) go BAZ)
      where go (x, y) elem = (FIZZ x y elem, BUZZ x y elem)

This usually starts out as a 'map'; then I find myself needing to append or discard some elements so I change it to a 'reduce'; then I find myself needing to propagate some info across calls, so I pair this on to the accumulator and discard it at the end. The end result is a sequential computation with mutable state; hardly a 'functional pearl'!

My point above was that we can't forbid 'reduce'; since it's a fallback when our calculation doesn't follow an established pattern; and that it's often useful to codify that pattern into a nice, generic function (using 'reduce'), and use that new pattern in our application code.

I haven't heard that before, but I've worked with several programmers who normally write very readable code, and when they write a reduce or fold, it's like they forget everything they know. They go super compressed and cryptic even if it's not their normal style. I can see why someone would want them to give it up.

Is it using tagless final or free monads?

Tagless final.

Reply on news.ycombinator.com