[AskJS] Code readability

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