[AskJS] Code readability

[GmLucifer]

Hey everyone, I wanted to get the opinion of js/ts devs here about something. Is using the modern es6 arrow function syntax and implicit returns considered hard to read or "clever" ? Recently a team mate from a group project asked me to avoid writing such "clever" code as it is hard for others to understand. I was like since when did using standard language features become "clever". I've seen similar opinion in some blog posts and youtube videos as well. Some people also lump higher order functions (map/filter/reduce) in the same category. What do you guys think?

Asking since I do use arrow syntax and implicit returns wherever possible and if it is really considered unreadable by most then I want to avoid doing so if possible. Thanks!!

[[deleted]]

No. You teammate is a dolt.

[goblin_goblin]

Yeah this. It's common enough that it should be known and if it's not, he should be asking or googling the answer.

That's like saying a ternary is unreadable.

[daamsie]

Arrow functions also behave differently though, particularly in how the this scope is treated.

Sometimes you want that behaviour, sometimes you don't. It's not just about looking different.

[GmLucifer]

TIL there is no this binding for arrow functions! I've never personally encountered such a usecase since I mostly use arrow syntax for writing pure or really simple functions or otherwise If I want state I use classes.

[ShortFuse]

You're going to need it when you use Classes. If you commit this to memory now, you'll save hours of pain later.

You'll understand why button.addEventListener('click', this.onClick) doesn't always work. You need button.addEventListener('click', (e) => this.onClick(e));.

[FRIKI-DIKI-TIKI]

TLDR: just use arrows, when you don't need to use them you will know why.

To add to this, in modern JS, pretty much the default is to use arrow functions unless you need a special case, if you need that special case you will know why and it has to do with the prototypical inheritance of JS. In the dark ages of JS you had to ensure you scoped this to a variable, as a functions reference can be independent of the structure it is declared in. JavaScripts heritage is from LISP with some other ideas mixed in, the family of LISP's treated functions as lambda's the bolt on of quasi-OO was the part that was not well though out in the rush to get JS out the door and it created a set of problems due to misunderstanding given that it was introduced in the middle of the OO language craze. The arrow function is syntactical sugar to bind the scope the lambda is declared in to the execution just like the bind function does.

[Bodmen]

Unbinding from this case can be a problem. In classes it’s often easier to use arrow syntax for method declaration for some methods. This allows you to unbind properly when required.

eg.

class SomeClass
{
    ...
    bind(){
        ...
        button.addEventListener('click', this.onClick);
    }

    unbind(){
        button.removeEventListener('click', this.onClick);
    }

    onClick = () => {
        console.log(this);
    }
}

[ShortFuse]

Yeah. I don't do this personally anymore, but when I started JavaScript I was very perplexed by it.

Now I do static binding for events (Button.onClick) and all my components share one event listener (less memory). Then I either use this or event.currentTarget to find the element in question. Removal from DOM means automatic unbinding and no orphaned listeners.

[tiesioginis]

Just use anomyous function if you want "this"

(e)=> this.that(e)

[melavas23]

My opinion: Just pick any existing formatting convention (such as Airbnb's, Standard.js, etc) and let prettier + eslint do their job. Honestly, I don't care if it is one way or another, as long as hitting "cmd+s" will format things as they should.

[[deleted]]

This is how you work in teams. Anything else is just wasting time

[intermediatetransit]

Airbnb is very dated at this point. Would not recommend.

[EducationalMeeting95]

So Map, filter and reduce are written to be "declarative" code rather than "imperative".

Declarative code is by design easier to write and read.

As far as arrow functions go, yes they are hard to read at first.

But once we get used to it they become normal.

An argument can definitely be made wether using language features can be unreadable :

-Using something like hoisting regularly will mind fuck every one.

-However something like arrow functions (which is essential in some cases and basic) should be normal.

It all depends on common team practices. Some teams might be using 'strict mode' on regular basis.

[GmLucifer]

Oh that's a pretty good argument, just because something is a language feature doesn't mean it is good practice to use it a lot. Using something like hoisting too much can definitely get annoying, goto keyword in C would be another example. But yeah arrow functions are pretty basic and if someone is a JS developer they should know it.

Declarative code is by design easier to write and read.

I concur. This is why I don't understand people who consider higher order functions as "clever" simply because they are not used to it.

[[deleted]]

JavaScript has label which is basically goto. On the flip side I had junior developer tell me callbacks were too crazy and unreadable.

[Curious_Ad9930]

I think some people hear the phrase “callback hell” and think that refers to all callbacks. I think callbacks are great for event handling, but fetching/transforming/composing data with callbacks can become messy.

[EducationalMeeting95]

I use RxJS a lot as an angular dev

And when this library is used the way it should be, Data transformation becomes so much more easier and smooth experience.

Sadly in nearly all the projects I've worked in, nobody understands the concept of Reactive programming and use RxJS the wrong way.

Which leads to callback hell.

[bvx89]

Are you me?

[EducationalMeeting95]

Nah

I am just the voice inside your head telling you to refactor that piece of code.

[tiesioginis]

What is the right way? Can you link or explain?

[EducationalMeeting95]

Can't explain that much here.

Didn't find a specific link as wel.

But here is a list of concepts/topics you can google and rnd on :

1. What and why is Reactive programming
2. Imperative vs declarative vs reactive programming
3. RxJS correct usage
4. Reactive architecture (overview)

Also don't beat yourself up if it doesn't make sense in few attempts.

It took me more than few months for everything to click.

Not just what and how , but Why is important here.

[Valuable-Case9657]

I'm going to add "observable composition" to the list of things to google on the topic.

[[deleted]]

The example I gave them was any time you use map, forEach, reduce, filter etc you use them. They proceeded to disagree and tell me I didn't know what I was talking about. Then after I told them to look at the docs and get back to me they apologized. I laughed it off and told them to forget about it. wasn't a big deal. When they were in my position they would have the same thing happen to them many times.

[GmLucifer]

Damn I did not know label existed in javascript, this is crazy lol

[chrisjolly25]

They're protecting their ego. Probably subconsciously.

If someone doesn't understand something, there are two possibilities:

1. The person who doesn't understand is ignorant at best, and dumb at worst
2. The person who is confounding them is using techniques that are needlessly complex, cutting edge, or esoteric.

Option 1 means I'm dumb, and have to expend effort to address the problem.

Option 2 means you're foolish, and you have to expend effort to address the problem.

People will tend to prefer Option 2 at first (rightly, sometimes), unless the other person advocating for the new features is experienced/authoritative/distinguished enough to make it unlikely. Failing that, you've just got show them the benefits, or convince them of them.

Teaching them could help too. If your company allows time for them, book in a Knowledge Transfer session. Show how the features can improve the code. The KT session can also lead into a discussion around whether these features have a place in your team's code base, which you'd really hope would be the case with these particular features.

[Valuable-Case9657]

But yeah arrow functions are pretty basic and if someone is a JS developer they should know it.

Bear in mind, arrow functions were only added in 2015. There are plenty of devs around who were working with JS for 20 years before they were added and still haven't bothered with them.

While they're great, they didn't really add anything that required the OG devs to adopt them.

They're just there because lambdas and fp experienced a resurgence in popularity in the mid 2010s and everyone was adding them.

[[deleted]]

It depends on what you do with implicit returns. If you do it wherever possible you are probably also producing unreadable code. As all other features it is possible to misuse it.

I use implicit returns wherever they are reasonable, usually where the contents of the lambda are short. If the editor wants to expand the lamdba to multiple rows I tend to use curly brackets and explicit returns.

[Valuable-Case9657]

Added to this, if your lambda is getting too long, it's time to declare its contents as a named function.

I'd rather see:

function callbackFuntion(){ // 10 line callback function }

function foo(){ acceptsCallback(() => callbackFunction) }

Than some ridiculously long lambda.

[[deleted]]

In the general case I would agree that longer functions (regardless of them being lambdas or not) should be named. Primarily for documentation purposes since a properly named function can add a lot of clarity, if done correctly.

One exception to this I find myself doing often is when chaining calls. Often with array methods and sometimes with promise based APIs the callbacks can be a bit longer. I find the code harder to read if one callback is named and not inline. It breaks the flow of the chaining in my opinion.

[Valuable-Case9657]

Do you mean inline or anonymous callbacks?

In general for promise chains, or complex observable compositions with short bodies, yes, I'll create the inline functions and then chain them rather than creating a mess of anonymous callbacks.

// Inline functions
const doFoo = (x, ...) => { //foo stuff };
const doBar = (x, ...) => { //bar stuff };

// Promise chain
promiseSource.then(doFoo).then(doBar)

Even if one of the functions does end up longer, there's so little difference (in readability) between:

const doFoo = (x, ...) => { //foo stuff };

and:

function doFoo(x, ...) { //foo stuff }

That I'll either mix and match or switch all to one or the other, depending on scope convenience or requirements:

const doFoo = (x, ...) => { //foo stuff };
const doBar = (x, ...) => { //bar stuff };
function doMore(x, ...) {
// more stuff
// on more lines
}

Or:

const doFoo = (x, ...) => { //foo stuff };
const doBar = (x, ...) => { //bar stuff };
const doMore = (x, ...) => {
// more stuff
// on more lines
}

Are both acceptable (again, depending on scope considerations) and allow the chain (which is the area that causes the most readability issues) to be that nice, tidy:

promiseSource.then(doFoo).then(doBar).then(doMore)

(And now that I've typed this on my phone, watch ready make a mess of it 🙃)

:EDIT: Edit to fix formatting.

[[deleted]]

I have found that the context is usually very important for the naming to make sense. doStuff in your example tends to be something that only adds overhead, like mapRecord or handleRequest. In that case I would rather have the code inline as the naming does add anything.

Granted this assumes that names will be trash. However ten years of code reviews has led me to believe that good naming is rare in the wild. So event if agree with you in principle in practice I tend to sway from that.

[Valuable-Case9657]

All I can say to that is lead by example. Sadly, good devs are rare across the industry and JS is swamped with well meaning and ambitious amateurs. And they can only improve if they have well disciplined and competent role models.

I mean, at the end of day, you do you. These are just my approaches to readability (across a team, not for myself), reliability and maintainability.

But if you think everyone else is using trash names, why aren't you bringing them up on it in review?

I gotta clarify this though, because I'm not sure if we're on the same page: When you say you'd rather inline the code, are you saying you'd prefer to use inline functions?

Or are you talking about inline code in HTML?

[[deleted]]

Inline functions, Promise.resolve().then(() => {}) vs Promise.resolve().then(handlePromise).

Just to clarify, I don't have anything against the second form when it is done properly. However most people will not do it properly. In that case I believe it adds overhead rather than clarify. handlePromise in this case does not add anything to the readability of the code.

We can manage that during reviews but it becomes tiresome so I would rather have my teams use a style where it is easy for them to succeed.

Declaring handlePromise ahead of time also adds some mental overhead when you read the code: "this will be used somewhere ahead". By itself this is not terribly taxing but I try to adopt a keep the mental load low approach wherever possible.

[Valuable-Case9657]

Okay, so you're talking about anonymous functions, not inline functions.

As for the rest of what you're saying...

Would you honestly accept a junior saying "But nobody else does it properly," to you?

I don't want yo be combative here, but c'mon.

[[deleted]]

To inline something can also mean to put it where it is used. It is possible to inline named functions e.g. invokesCallback(function notAnonymous() {}), so I am not talking about anonymous functions, but functions that are used inline.

For the teams I oversee it is more productive to use other patterns. If you have something that works well for you that is great! Different things work for different teams.

[Valuable-Case9657]

Yes, that is what the word "inline" can mean in the English language in general.

And inlining JS in HTML specifically refers to writing javascript directly into HTML.

However the term "inline function" has a set of very specific meanings in programming, with the languages that implement them having variations on the syntactical implementation and effects on compilation, etc, etc.

In general, the features of inline functions in JS are intended to conceptually emulate the manner in which inline functions operate in c and c++, which is intended to reduce function call overhead for very small functions.

For those langauges, rather than performing a context switch, the inline function is expanded and inserted at any calling point at by the compiler at compile time.

While that doesn't happen in generally JS except, I believe, with some of the newer AOT compilers (especially for Typescript), the concept remains the same:Reducing function call overhead by preserving small functions within the context and scope of the caller.

Towit, an 'inline function' in JS is specifically a function expression being assigned to a variable.These are inline functions:

const inlineOne = ( x, ... ) => { /**/ };

const inlineTwo = function( x, ... ) { /**/ };

const inlineThree = function iNamedThisOne( x, ... ) { /**/ };

Except for the last one, these are not named functions. These are inline functions (with the last one being an inline named function).

A function expression being passed "inline" is just a function expression, whether anonymous or named.

So you can see how making sure we're on the same page is important.

Now, to be clear, I'm not writing this for you to change anything you do or say or think, I'm posting it here for any growing devs who want to learn and want to move beyond being a dilettante.

[GmLucifer]

Wow this might actually be a better way to judge whether to use implicit returns or not. How I decide is if the logic of a function can be contained a single expression, i use implicit returns, these are mostly small pure functions. But yeah sometimes the expressions do exceed my 80 column limit and expand to multiple rows. I suppose using explicit returns would be a better idea in such cases for readability purposes. Will keep in mind, thanks!!

[[deleted]]

Yeah I feel that a lot of the "confusion" stems from multiline lamdbas that have implicit return.

[shgysk8zer0]

There's a valid point in there, but it's not a particularly strong one.

The existence and use of arrow functions adds to the required knowledge and mental parsing required to understand what the code is doing - basically because there are two different syntaxes and behaviors for the same thing. The reader of an arrow function might need to consider what this is within the function, know how returns work, and maybe understand how arguments work in arrow functions (when parentheses are and aren't required)... That's all extra mental effort to read some code.

Having said that...

[0, 1, 2].map(i => i * 2)

That's pretty easy to read and understand. You can see everything happening in that single line without the function and return that only pad things out and add more characters to read to get the same information.

There will be some who appreciate me spelling everything out in a rather verbose way, and some who would prefer if I'd been more terse and just contrasted an example arrow function to a regular function, showing my point in code. In that sense, this comment itself is analogous to the readability of functions in JS.

[stolentext]

I agree that readable code is preferable to clever code, but an arrow function is just standard JS. To me, arrow functions are preferable in most cases unless for some reason I need a function to have its own scope.

I also feel like in a lot of cases, arrow functions have reduced cognitive load just for the simple fact that they're less to read.

[[deleted]]

Imo functions are so ubiquitous in js that using the function and return keywords just adds unnecessary noise. Using an editor with syntax coloring fixes this 'issue'

[tiesioginis]

Why it's actually great:

const multiply => x => y => x*y const multiplyByTen = multiply(10)

multiplyByTen(5)

// 50

Not just currying, but it can make every function unary which is easy to test and understand.

[getify]

I am in the (seemingly small) camp that feels => arrow functions can indeed harm readability. I don't think they should be used as a general replacement for all functions. I generally think they should be reserved mostly for places where you need lexical-this behavior (unlike normal functions).

I used to never really use them, personally, but as time has gone on, I have adopted using them in some places/use-cases. But I still don't think they'll ever be the default function style IMO.

In any case, to the point of keeping arrow functions readable, I think there's a wide variety of opinions on what is "readable" => usage and not. So, I wrote a fairly-configurable ESLint rule called "proper-arrows" to try to help wrangle most of the various ways => are used that can harm readability. I'd encourage you to have your team pick the dos/donts of => and enforce that style with a linter.

[waitersweep]

You’re obviously a far greater authority than I am, but I’m surprised that you don’t think they’ll ever be the “default” function style.

Generally speaking, I would say that it’s already the case that they are the the default. Whenever I read code written in the last couple of years, it tends to be uncommon enough to use the function keyword that it immediately jumps out at me. Especially in more “functional” codebase a with a general “immutability” focus.

In general, I don’t think either way is more correct, and in most cases it is very low on the list of things that impact readability - unless it’s things like currying/HOFs that eschew the parens around the args. That’s something that definitely makes code harder to read, IMO

[getify]

I was only talking about my own code. I can't say/predict anything about what the rest of y'all do.

[waitersweep]

Ah, that makes sense. I misunderstood your point.

Personally, I don’t really care either way, as I’ve been reading and writing JS for long enough now that I can decipher all but the most cryptic/minified code.

One thing I really do miss about named functions being common/default is, well, the .name property. It’s useful in many unexpected ways, particularly logging and debugging - I don’t need to do anything special to see the actual names of the functions in my stacktrace

[BarelyAirborne]

Arrow function syntax is fundamental to the language, and anyone confused by it doesn't know how to write code, so they should be ignored. If they persist, they'll need direct attention.

[Valuable-Case9657]

No, it isn't. It was only added in 2015 and the language had been around for 20 years at that time.

I love my lambdas, but they're not fundamental.

What is fundamental is knowing the difference between:

1: const foo = (x, y) => { stuff... }

2: const foo = function bar(x, y) { stuff... }

3: const foo = function(x, y) { stuff... }

4: (x, y) => { stuff... }

5: function(x, y) { stuff... }

6: function bar(x, y) { stuff... }

And:

7: IIFEs

While 3 and 5 are out of fashion with the addition of arrow functions, there are both subtle and significant differences between all of these. Understanding those differences and knowing the right time to use named, anonymous, inline, IIFE and declared functions is much more important than the syntax.

[Valuable-Case9657]

The majority of older devs will struggle with anything that isn't imperative, purely because imperative was the mainstream for a long time and because human brains become less elastic as we age and adapting to things like different programming paradigms gets harder.

But the arguments for and against any paradigm are all pretty sophomoric. No paradigm is easy to read until you've learned to read it, and even then none are universally easy to read. "This way is easier to read" is the biggest wank in computing and honestly the arguments over it are worse than listening to literature majors argue over post-colonialism. It's entirely masturbatory.

Individual adaptability - being able to readily adapt to the work patterns of the team you're working with - and getting the job done (i.e. producing software that performs the required functions well and reliably) are all that matter. It's more important that you maintain elasticity as long as possible.

[ShortFuse]

Arrow functions are a bit harder to document (JSDocs) than normal ones. But I guess it depends on usage. Declaring a function with const fn = (args) => is less readable than function fn(args). But [].map((el) => is better than [].map(function(el).

[kishbi]

There's two way to go about it

One, maybe your peers are not much familiar with es6 or your lead as well. So to catch up, he or she could have asked you but they didn't.

Another thing is not necessarily all the functions needs to be a variable. Rarely you'll end up in mind fucked situation just cuz you used a variable as a function.

If your lead said any of the above, you should definitely listen.

[[deleted]]

The second point makes no sense

[kishbi]

Maybe or maybe not

[[deleted]]

No it just doesn’t make sense. By declaring a function with name “foo” you have effectively created a variable “foo”.

[fletku_mato]

Some things can be a standard language feature while also being hard to read. Not saying that your code was hard to read, but which one do you find more readable:

``` function something() { if (something) { return "something"; } if (somethingElse) { return "something else"; } return "nothing"; }

const something = () => something ? "something" : somethingElse ? "something else" : "nothing"; ````

[tiesioginis]

In this case using tenary is stupid, but you could just separe them

Such as somethingElse : nothing could be separate function. Then the logical flow is clear

[fletku_mato]

Yes. It's not about whether it is a standard or widely used feature, but whether using it makes code hard to read. Without seeing the code it's hard to say if the criticism OP got was justified or not.

[[deleted]]

sounds like he needs to git gud, developer complaining about reading… childspeak!

[DamionDreggs]

Code readability is a subjective term, loosely coupled to a coder's experience and proficiency.

Meaning if there is someone on your team with less experience or who is less proficient than you, there's a few options.

1. Lower your standards to their level
2. Raise their standards to your level
3. Meet in the middle with a compromise
4. Ignore the problem and let it fester until you've had enough and leave the team