Skip to content

Аvoid mental mapping #336

Open
Open
@NikitaIT

Description

@NikitaIT

https://github.com/ryanmcdermott/clean-code-javascript#avoid-mental-mapping

Remove or edit rule as:

Bad:

doSomeStuffWithLocation(location);

function doSomeStuffWithLocation(l) {
  doStuff();
  doSomeOtherStuff();
  // ...
  // ...
  // ...
  dispatch(l);
}

Good:

doStuff();
doSomeOtherStuff();
doSomeStuffWithLocation(location);
dispatch(location);

function doSomeStuffWithLocation(location) {
  // ...
  // ...
  // ...
}

I can't think of straight away how to reflect this correctly, but the way it is brought up is confusing.

This lambda rule is misleading.

const locations = ["Austin", "New York", "San Francisco"];
  // This code is not clean.
locations.forEach(location => { 
  // This code is not clean, too.
  doStuff();
  doSomeOtherStuff();
  // ...
  // ...
  // ...
  dispatch(location);
});

In fact, in this case, we have to move the function out by making it named. Or apply a different refactoring.

const locations = ["Austin", "New York", "San Francisco"];
locations.forEach(doSomeStuff);

A correct example would be something like:

Bad:

const locations = [{ id: 1, name: "Austin" }, { id: 2, name: "New York" }, { id: 3, name: "San Francisco" }];
const locationIds = locations.map(location => location.id);

Good:

const locations = [{ id: 1, name: "Austin" }, { id: 2, name: "New York" }, { id: 3, name: "San Francisco" }];
const locationIds = locations.map(x => x.id);

In this case, we consider functions as a workflow, and it does not matter to us exactly how the parameter is named.

locations // map locations
  .map(x => x.id) // to id

Or a more complex example:

Bad:

users // map users
  .flatMap(user => user.orders) // to flat user orders
  .flatMap(order => order.payments) // to flat order payments

And the developers read it as:

users
 .flat ... user => user.orders
 .flat ... order => order.payments

Good:

users // map users
  .flatMap(x => x.orders) // to flat orders
  .flatMap(x => x.payments) // to flat payments

And the developers read it as:

users
 .flat ... .orders
 .flat ... .payments 

Or just:
users.orders.payments 

Using an abstract x reduces the read overhead since the beginning of the expression x => x. ignored.

Many developers may find this strange, but we never actually read the names of variables in lambda functions, and we almost never write lambda functions longer than a couple of lines.

They also usually ask what to do with nested functions. It all depends on the situation here, but my adherence is that if nesting appears, it is wise to move it out of the chain. As in https://github.com/ryanmcdermott/clean-code-javascript#functions-should-do-one-thing

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions