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.
> 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.
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
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.
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!
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.
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.
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.
> 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.
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.
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.
> 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.
'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.
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)
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:
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.
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.
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.
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.
I agree Rubocop is heavy handed in the rules it hands down by default, its really useful as a formatter, but we have an internal gem at work which contains a set of less strict rules around things like complexity and method length to try and get away from having to play code golf all the time to keep Rubocop happy.
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
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.
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.
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.
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.
Challenge accepted. Here's how far I've got so far:
$l=[nil,nil,nil,nil]
TracePoint.trace(:call) do |tp|
$l << tp.self
$l.shift
end
class Lol < BasicObject
def initialize(f)
@f = f
end
def method_missing(m, *a)
# puts $l[0].inspect
$l[0].send(m, @f, *a)
end
end
class BasicObject
def ►(&blk)
::Lol.new(self).instance_exec(&blk)
end
end
module FooBar
def self.foo(a)
if a < 0
a.► { bar -1 }
else
a.► { bar 1 }
end
end
def self.bar(a, b)
(a*b).► { puts }
end
end
FooBar.foo(4)
I've almost written "|> case do" several times and then in each case have decided that it makes sense to a separate private function (with function head matching for the different cases).
I don't think it's dynamic, just macros. a |> foo() |> def do ... end just gets macroexpanded to foo(a) |> def do ... end and then def foo(a) do ... end.
This is fun, because it shows just how pervasive function application is in programming. You can build whole programming languages on just function application (which is what the Lambda calculus is about).
