[karolkozub]

I was raised on programming contests and it took me many years to realize that clever code doesn't equate good code. In the vast majority of cases readability is more important than cleverness, performance, abstraction, and adhering to imaginary rules. The best measure for the quality of the codebase is whether at a glance you can understand what's going on.

[dkarl]

> 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.

[aprdm]

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)

[dnautics]

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

[rkangel]

> 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.

[dnautics]

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

[thedanbob]

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!

[higerordermap]

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.

[lawn]

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.

[nemetroid]

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.

[barrkel]

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.

[hultner]

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.

[barrkel]

> 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.

[hackerfromthefu]

Are the downvoters perl programmers?

[antfarm]

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.

[throwaway_pdp09]

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.

[dkarl]

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
  )

[mtberatwork]

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

[rkangel]

> 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.

[bryanrasmussen]

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.

[dreen]

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().

[chriswarbo]

'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.).

[dkarl]

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.

[chriswarbo]

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.

[dkarl]

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.

[hderms]

Is it using tagless final or free monads?

[dkarl]

Tagless final.

[hprotagonist]

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

... in 6 months when you’ve forgotten 80% of the context.

[dnautics]

Elixir is pretty good at this. I've found it really easy to do a (useful) drive-by pr on someone's open source code, for example, so that's with almost no context whatsoever.

Nobody writes code like the op though (I'm pretty sure it's satire)

[imperfectcats]

I think having a tool to standardise formatting is huge, and a lot of what makes Elixir code easy to read is mix format.

That and the fact most of the community started out with similar ideas on good code, so there was less bikeshedding around formatting.

[Fire-Dragon-DoL]

I have to disagree on this point.

Formatting could be beneficial, but it gets abused so fast that it becomes a problem.

First thing, no one challenges the choices made by the formatter, which is a problem in the long term. The other thing is how far the formatter goes. Rubocop in ruby is a clear example of this, they went way too far with it and producing a readable rspec test is impossible without violating at least one of the rules.

A few days ago, I ended up writing something along these lines:

```

def something err, obj1 = dependency1.call(someargs) return err, obj1 if err.nil?

  err, obj2 = dependency2.call(obj1) 
  return err, obj2 if err.nil?

  err, obj3 = dependency3.call(obj2) 
  return err, obj3 if err.nil?

  err, obj4 = dependency4.call(obj3) 
  return err, obj4 if err.nil?

  err, obj5 = dependency5.call(obj4) 
  return err, obj5 if err.nil?

  [nil, obj5]

end

```

This is a pipeline, to a human being it looks simple because the "return line" after reading the first time and understanding it's an early exit in case of errors, it's identical in all 5 steps. Human brain just excludes those returns after having read the first one.

Rubocop however claims that there is too much complexity going on here due to 5 if branches. That's a machine reading the code.

If I have to rewrite the code according to rubocop standards, it ends up being a lot less readable and with a lot more indirection for no particular advantage.

I find it funny, we use styleguides to ease human interactions with code, but we let the machine evaluating that. It's problematic, the machine doesn't see the code as us.

[zapnuk]

Edit: My bad didn't recognize the parent comment was about Ruby, and that the code isn't elixir by a long way.

Correct me if I'm wrong but I think there are 1 (and a half) other ways you could write it - that don't result in branching

1:

  def something(args) do
    with {:ok, obj1} <- depencency1.call(args),
         {:ok, obj2} <- dependency2.call(obj1),
         {:ok, obj3} <- dependency2.call(obj2),
         {:ok, obj4} <- dependency2.call(obj3),
         {:ok, obj5} <- dependency2.call(obj4)
    do
      {:ok, obj5}
    else
      {:error, msg} -> {:error, msg}
    end
  end

1.5: Use a try/rescue block where you match {:ok, obj} and then catch Match errors.

[Gaelan]

I think your parent was writing Ruby, not Elixir. (Unless this is Ruby's new pattern-matching syntax that I'm not familiar with.)

[Fire-Dragon-DoL]

Indeed if you look at my ruby code, it tries to be a with statement ;)

[rkangel]

The parent is referring to rubocop, so I think they're talking about Ruby.

The Elixir formatter sounds much less strict, and I have very little problem with it.

[whitexn--g28h]

mix format doesn’t have complexity rules. It will never change the semantics, it’s just there to make sure all the braces, brackets, commas, pipes, etc. end up in the same place so that it’s easy to scan a file.

mix format is not a linter it’s a formatter.

[Fire-Dragon-DoL]

Indeed, that's why it's acceptable.

Even in that case, sometimes it breaks organization for some code that if kept as is would be more scannable to a human.

[andrewem]

Are your objections to Rubocop's dictates in tests only (or primarily) about the complexity rules?

The complexity rules have always struck me as being of a whole different category thnan Rubocop's other rules. As you note, they're especially frustrating in tests.

Edit: by "the complexity rules" I mean https://www.rubydoc.info/gems/rubocop/0.27.0/RuboCop/Cop/Met..., rather than the ones about variable naming, line length, etc.

[Fire-Dragon-DoL]

My problem with Rubocop is both in tests and in normal code. The rules about complexity hit both tests and production code, while the rules about tests obviously impact only that (experience limited to RSpec).

A senior developer won't need Rubocop, it's a blocker rather than an improvement. In the rare occurrence where you have an undisciplined senior developer, it's worth exploring training or re-evaluating the standards in place.

All in all, I keep thinking this is a problem of culture, if it's addressed there the value in Rubocop decreases drammatically.

That being said, Elixir formatter is "ok-ish". I didn't have the same problem with it because it doesn't overstep the boundaries of styling. It did remove valuable structure of the code for the sake of formatting standardization, so again it's actually doing damage, but at least it doesn't force you to write code that is more cryptic to a human for the purpose of pleasing a machine.

[girvo]

It's not even just Rubocop -- I've had this exact problem with any heavy-handed auto-linter/formatter that complains loudly about complexity without being able to understand context

[red2awn]

Exactly. I worked at a ruby shop a while ago and the rubocop lints makes you write code and test less readable than they can be.

[dnautics]

Eh, I think there are more important things, like if you pass a variable to a remote function, it doesn't ever mutate underneath you when you try to use it later.

[mercer]

Agreed. I maintain a whole bunch of codebases, many of which are a few years old, and it's so much easier to dust of an Elixir project (pre-formatter) than, say, picking up on a Wordpress or Drupal project.

[setr]

6 months..! Give me a few days and I'll have forgetton anything I didn't touch at least 3 times :-)

[darkerside]

You are right in the context of software built for ownership but a team, typically in a commercial context.

In other cases, side projects with an emphasis on learning, coding for fun, practicing, or just doing something neat, the clever code can meet other subjective standards for "goodness".

I'm sure most people don't care, but that doesn't make it less true.

[dilap]

Yeah there's definitely a place for both.

That said, the older I get, the more I appreciate obvious code...

[darkerside]

Me, too. But I'd posit that's because we've written enough of the clever shit that we don't need to learn, play, and experiment anymore, at least to earn a decent living.

[timoth3y]

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

I agree with you in spirt, but I would change "you" to "a new teammate" in the above sentence.

[cutler]

I was raised on Perl golf by Tim Toady so I have to respectfully disagree.