"Use names to convey purpose. Don't take advantage of language features to look cool."
I can't say enough about this. Please write code that is easy to read and understand, not the most compact code, and not the most "decorated" code, or "pretty" code or neat because it uses that giant list expression or ridiculous map statement thats an entire paragraph long.
Similarly what bugs me is when I receive a pull request where someone has rewritten a bunch of code to take advantage of new language features just for the hell of it and that did not lead to an increase in clarity.
I guess its in vogue now to add a lot of bloat and complexity and tooling to our code. "Use the simplest possible thing that works." Tell that to the Babel authors with their 40k files...
Using intermediate variables is one of the most underrated tools to make code more understandable. It's the definition of something completely unnecessary from a technical standpoint that is all about conveying meaning and clarity to other programmers. And it can be used to help group and "modularize" chunks of code within a routine without necessarily going to the extreme of pulling out a separate subroutine, which can be overkill in some circumstances.
What's better than comments to describe what the code does? CODE that describes what the code does. (Let the code describe WHAT the code does, and if necessary, the comments describe WHY the code does it like that.)
In C++, if I use an intermediate variable to decompose a complicated expression into easier-to-understand sub-expressions, I like to make the intermediate variable `const` to emphasize that it's an intermediate component.
beautiful has it's value too. I prefer a code that creates paragraphs using an extra line break. A single comment can
explain what each paragraph means. Reading (simply) a file is "open it, use it, close it", in my code it's three paragraph of code.
I hear this a lot, often in the for "Good code doesn't need comments", but I'm more than a little skeptical of this view.
I need actual examples to get on board, and not just the usual "idiot" programmer strawmen - actual examples that aren't obviously unreasonable.
My main reason for skepticism - Full, proper English sentences are capable of a lot more nuance, and precise semantics than whatever the programming-language syntax might support. I agree code can be written with more clarity, but it cannot substitute actual text, i.e. Code is not documentation.
I agree, intermediate variables are a highly underrated technique that can make code more understandable at a glance. What is annoying is that quite a few IDEs flag intermediate values that can be removed as a possible error, which probably discourages quite a few beginners from using them.
I think people underestimate the cost of vertical length. It's less obvious in small examples, but there's a huge difference in readability between a class or function that fits on one page and one that doesn't, so it's well worth making individual lines a bit less readable if it means you need less of them.
Our teacher of programming told us something like this: if your program has so many lines it cannot fit on one screen, it contains at least one bug, so don't make them, keep them short. I try to make all projects made of small files, where each one can fit on one screen. Great for comprehensibility and productivity (less scrolling). Sometimes, a procedure gets longer than one screen, so what one needs to do is to put as much source as possible on a line, so that the entire thing fits into one screen.
Making individual lines a bit less readable is well worth it if making individual lines a bit less readable means you need less of them. ("Worth it" is a construct I don't really understand the grammar of - I'm a native English speaker)
I recently got a code review that in several places suggested I switch to the new Java 8 stream API [1]. I just flatly responded that it was far less readable, even if I could condense a half-dozen lines of code down to one. Where I can quickly scan over a foreach loop to get the jist of what it's doing, I have to closely examine each call in the new approach to have any idea what it's doing.
Stream based programming is a paradigm that with some training and proper code indentation is much, much faster to read than a nested for loop. Once you get used to it,you can literally fast-scan code written in this style with the confidence that you are not missing anything.
Also, assuming you do not use mutable state, it also has the advantage of being easily parallelizable without any code changes. (As well, as easily combinable, throttlable, samplable etc)
But it _definitely_ has a learning curve (like Calculus) and one needs to invest time in it to get used to this paradigm. Time must be spent in learning all the various operators and how they can be effectively leveraged. Stream-based code is certainly not something you can read off the cuff.
The good news is that this is valuable time investment since all stream/observable libraries across languages have a large set of identical operators and shared API surface - the cross platform applicability is fantastic.
Once you know stream based code in language A, you can automatically read stream-based code in language B, without even knowing the syntax of language B.
I love observable stream libraries like RxJS. It makes async programming so easy! I could never write async JS code effectively with callbacks or even promises. Too much state to keep in my poor head.
We now even have standards like ReactiveX coming out that are attempting to define a cross-language/platform spec for observable flows and standardized patterns on how to create new stream operators, etc.
At risk of a terrible analogy, I consider Stream based code to be something of a mini-language like Regex. (but easier to read). No one who hasn't put in time understanding regex elements will ever follow regex patterns. Ditto for stream based stuff - you need AOT learning, but the rewards are worth it.
I honestly think most programmers fluent in Java 7 programming, can guess this is finding "grocery" type transactions, sorting by "value" transaction property in descending order, extracting the transaction ids, and returning the result as a list. ("map" could be confusing, as this usage is borrowed from functional programming.)
May take a while to learn the performance and memory usage characteristics, and how to correctly write code in this style, but I'm not buying that it is difficult to "read off the cuff."
Err..yes, Java 8's filter, sort, map and collect are the most basic stream operators and you are correct that even folks un-used to this style can determine an understanding. But in code that uses observable stream-based programming you nearly always use more complicated operators like flatMap, merge, skipWhile, takeUntil, combineLatest etc. Without spending time learning these operators and how streams work, one is always going to scratch one's head. (Unless you possess an FP background)
Even in Java 8 plain streams, the Collector is a powerful paradigm for grouping operations with a large amount of variations and it takes (at-least it did for me) some involved time learning how to effectively leverage this in day to day code.
seems to be net improvement to me. It reads like SQL, and eliminates many causes of error (wrong indexing variables, off-by-one, copy-paste error in boilerplate).
Yes it requires learning several new concepts, but in the end it pays off.
BTW I wouldn't switch old code to this style, because there never seems to be time for that. But new code written like that is perfectly OK IMHO.
EDIT: I really should've checked the code more carefully - the initial version had no indexing variables. Still, it reads better and has less boilerplate.
While some people may like to read code that looks like SQL, I've found Java 8 features like this are poorly supported by the debugger, so debugging stuff like this tends to require "horse whispering" or rewriting the logic into something that can be stepped through.
I think it reads better with intermediary variables. Granted sometime you go to name an intermediary variable that does two things (just like male_siblings = (2 * (X + Y)) if I have a brillant idea for the variable name.
As a functional programmer (Haskell, Erlang) I can honestly say that after years of FP I think differently about how to describe the intent of my code. It's more difficult for me to understand a foreach loop with mutable state than a foldLeft.
I think it's a familiarity problem. I felt the same at one time, but once I became more familiar with the operations, I found the stream API much easier to scan.
I was asked at work to explain why I prefer the stream library - other developers have lagged a bit on usage.
The first reason is, the method calls are designed to only do so much, and are named after what they're designed to do. Filter is for filtering. Map is for mapping. I can read the first word of each line to get an idea of what it's doing (filter sort map).
Another bigger reason for me is: you know exactly what code is generating those transaction ids. If you are interested in how the transaction ids are generated, it is damn obvious which code you need to look at. And if you aren't interested in how the transaction ids are generated, it is damn obvious which code you can safely ignore.
In comparison, what does a for loop do? It does all sorts of things, oftentimes several different things at once. As such, I leave for loops for more involved processing, and use the stream API for straightforward transformations.
As someone who could very easily be on the other side of that code review (and I'm preeeettty sure I'm not in this case?) I feel obliged to at least try to provide a counterpoint :).
So I agree that enormous blobs of unreadable crap are indeed unreadable, and that regardless of how neat and functional your code is, it can still be complete gibberish to most people. That being said, long chains of streams can be broken out quite nicely by using intermediate names:
vs. the first code block under "Figure 1" in the linked article.
For me at least, it helps keep code from getting too unruly: you only have one 'thing' you can do in a filter, map or sorted call, unlike in a foreach loop where anything can go. So my thesis is this: using streams I can quickly scan over the function/stream names to get the gist of what it's doing, but using foreach loops I need to closely examine each line to have any idea of what's happening :3 (e.g. people love abusing labeled breaks [1] in our codebase, as well as excessively modifying input parameters w/o documentation, so I might be a bit biased against for loops)
I find the code in the linked article much more readable than that. You've introduced a huge amount of noise that makes it hard to see what the actual operations being performed are.
I agree to an extent ... this example is pretty contrived, but when you start getting around ten filter/map/groupby operations in, it gets a little difficult to follow what's supposed to be happening. So typically, my first step towards breaking it out into a method is separating out the individual streams like above. As is mentioned in a cousin comment, it also looks a lot nicer with type inferencing, but alas we are stuck with the verbosity of standard Java 8 for now.
This is a place where local variable type inference really comes in handy for cutting down the noise of the type declarations.
var descendingTransactionsByValue = comparing(Transaction::getValue).reversed();
var groceries = transactions.filter(t -> t.getType() == Transaction.GROCERY);
var sortedGroceries = groceries.sorted(descendingTransactionsByValue);
var transactionids = sortedGroceries.map(Transaction::getId).collect(toList());
Look, you're basically just saying you don't want to learn anything new.
"I just flatly responded that it was far less readable, even if I could condense a half-dozen lines of code down to one."
Express the exact same thing, in 1/6 the code, and somehow the shorter code takes longer to read and understand?
You don't even attempt to express why you find the shorter code harder to understand. Or show code written both ways, to argue why one is better than the other. Or anything at all to support your claim.
Based solely on what you're telling us here, I'm siding with your code reviewer on this one.
> Express the exact same thing, in 1/6 the code, and somehow the shorter code takes longer to read and understand?
I think people need to differentiate between code review and code scanning. I look at code completely different when someone is pointing to it than when I'm scanning through a file. A ternary operator isn't tough to read when you're already looking at it. However, scanning through a file I'm going to have a lot more difficulty spotting a `?` than I am the indentation of an if/else block. I'm more than content to take 5 lines instead of 1 to spot it 2 months later when I don't know what I'm looking at.
I suppose to some extent this is a matter of taste, but I've usually found the streams api to be clearer and easier to understand, although perhaps there's a bit of a learning curve. I think the win is especially large in cases where you replace an anonymous inner class with a lambda, imho. At my previous job, we had all gotten quite familiar with Apache's CollectionUtils, but java8 almost obviates the need for that entirely.
hopefully they flatly responded that you're wrong.
more generally, if you can condense 6 lines to 1, that change will almost always improve readability. ofc, you can create golfed examples where this is not the case, but those are outliers. i'd say that if you are arguing in favor of code 6x as long as an equivalent implementation, the burden of proof is on you. and being used to a more verbose syntax because you've used it for years is not a valid defense.
Also, there's an unstated reality here that the person who sets the standards is a senior person (in the company, in his career, in the industry) and may just be letting personal judgment and past success - good in its time, but not necessarily timeless - override his judgment about a legitimately new and better way of doing something. This is tech after all: progress is valuable, and objectivity is hard.
Maybe I've been shaped by my history... but I've been writing core code for other devs to extend for 5 - 8 years by now. And hell, the smart code I've written is necessary, sadly, but impossible to maintain. I've written non-blocking, lock-free holy-balls pieces of code, but there are very few people I know who could maintain that kinda code.
The best code I've written is when a junior can hit me up and go "Yo, I've tried to implement this feature here. I've picked up on those patterns over there to get started. I'm not sure about those two parts, can we talk about those a little? I think the rest of the code I've done is good, let's look at that though. That part over there just looks weirds, I'm not certain it'll work right."
Overall, I think smart code not a good thing to do. 90% of your code, or more, should just obviously work because it follows "the pattern".
I can't say enough about this. Please write code that is easy to read and understand, not the most compact code, and not the most "decorated" code, or "pretty" code or neat because it uses that giant list expression or ridiculous map statement thats an entire paragraph long.
Similarly what bugs me is when I receive a pull request where someone has rewritten a bunch of code to take advantage of new language features just for the hell of it and that did not lead to an increase in clarity.
I guess its in vogue now to add a lot of bloat and complexity and tooling to our code. "Use the simplest possible thing that works." Tell that to the Babel authors with their 40k files...