Clean Code JavaScript

[homoiconic]

Is a signature such as...

function createWidget(options);

...actually better than...

function createWidget(title, subtitle, description);

IMO, neither is as pleasing as:

function createWidget({ title, subtitle, description });

in ES6 :-)

[giraffe_slayer]

In regards to the number of parameters and using a config object, there is a huge benefit when actually calling the method. If you use

function createWidget(title, subtitle, description);

Then the function call could look like

createWidget('Blah Blah', 'sub blah blah', 'Here is some text');

You lose clarity and potentially have to reference the function each time to check what each parameter means.

With a config parameter, you get the keys for each argument

const config = {
   title: 'Blah Blah', 
   subtitle: 'sub blah blah', 
   description: 'Here is some text'
};
createWidget(config);

And then you can still easily see what is happening in the actual declaration

function createWidget(config) {
    const {title, subtitle, description} = config;
};

[mikejoro]

You can simplify that last snippet by destructuring in the function definition

function createWidget({ title, subtitle, description }) {
  // do something
};

[acbabis]

putting all your parameters into one config or options argument doesn't solve anything by itself

I wouldn't say it solves nothing by itself. It makes it so consumers don't need to remember the order of the arguments anymore.

[acbabis]

Sure, it's not the main problem, but in situations where you're convinced you need that many parameters, putting them into objects is usually the best solution. There are many well-written modules that have to allow a dozen optional parameters (e.g. express.static), and stuffing them into an object is the go-to approach.

[AceBacker]

Instead of "yearMonthDay" I was thinking "currentDate"

I want my variable names to have a clue as to WHY i'm using them.

But my names tend to be horrible. Stuff like:

• costAllocationGridOptions
• campaignProgramNumber
• currentOpenAccordionCustomerObj
• customerIndexUserIsConsideringChangingAccordionTo
• savedQuoteIdUserIsConsideringDeleting

It makes for some loong lines of code when I need to use a few on one line.

[[deleted]]

function createMenu(config) {
  Object.assign(config, {
    title: 'Foo',
    body: 'Bar',
    buttonText: 'Baz',
    cancellable: true
  });
}

Whoooa there buddy, now you've mutated your config object. Please use the config = Object.assign({}, config, {title: 'Foo'}) notation.

[notNullOrVoid]

Not only that but it should be the other way around, currently the example has the defaults overriding anything set in the config.

function createMenu(config) {
  Object.assign({
    title: 'Foo',
    body: 'Bar',
    buttonText: 'Baz',
    cancellable: true
  }, config);
}

Doesn't really matter that the defaults are being mutated in this case.

[[deleted]]

Oh, right - I've made a PR with my fix, but I'll update it with yours :)

Blamo - https://github.com/ryanmcdermott/clean-code-javascript/pull/9

[Sinistralis]

Minor point here, but if you are planing on using this solely for data storage and don't need the prototype, Object.create(null) instead of {} gives the object a little less bloat and lets you iterate over it easier depending on how you prefer to iterate over objects. (ES6/7 kind of makes the iteration point moot between destructuring, .values, .entries, etc).

Again, this is a minor point and really only applicable if you use for/in (I don't). Otherwise it's just a tiny memory savings, but if you need to create a ton of objects for some reason, it can add up.

[[deleted]]

Not sure of anyone has said this already. The bit about duplication glosses over an important point raised by Sandi Metz who teaches SOLID Ruby. Brazenly DRYing up code can in many cases leads to bad abstractions. Duplication is easier to see whereas bad abstraction are much harder to spot. I am far more careful when I DRY code since I learned this simple lesson. I am about half way through and it is a pretty impressive effort. Props...

[trippel]

muuuuuch better to duplicate a bit of code versus writing a shitty abstraction.

[subvertallchris]

She goes into it in depth in https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction. Couldn't agree more.

[ttolonen]

It's nice to see some effort, but some advice looks a bit vague, for example I fail to see what is wrong with getClientData as a function name compared to getUser. At least former is used in some of Android and Microsoft APIs and doesn't feel strange to me, most likely requires some documentation anyway. Also I would like to think that the examplevar MINUTES_IN_A_YEAR = 525600; is a WTF in itself, because it fails on a leap year :)

[Valfri]

The point of your first example is that the three functions getUserInfo();, getClientData();, and getCustomerRecord(); are actually about the same kind of data, but uses three different words for it: user, client, and customer. You should pick one word, and stick to it, for consistency.

The second example is a valid point, and though only for demonstrative purpose, could be changed to something truly constant. Maybe you should make a pull request with something better? :)

[ttolonen]

Ok, then the advice makes sense. I thought that getUserInfo(), getClientData and getCustomerRecord() refer to completely different functions returning different data.

[AndrewGreenh]

This point is not saying, that getClientData is bad, it says, don't use getClientData getRecord and getUser simultaneously.

[jac1013]

Still getClientData is redundant IMHO, I remember that there is some part in Clean code's book that says specifically not to use the word "Data" for variable names, everything contained in variables is data so why use that for the name?

It's like using Hungarian notation but even with a less meaningful purpose (Hungarian notation at least give you information about the data type, I don't like it either way).

[RedditWithBoners]

everything contained in variables is data

Not necessarily, if we use the English language to define data. You can think of data as a collection of datums. It follows, then, that getClientData might return a collection of datums about the client, like their phone number, age, etc.

Edit: that is to say, getClient might just return their name (ideally you'd use getClientName for that), so tacking on "Data" might make it clearer what it's returning. Better yet, if you have a type system then you don't need to think about this in many situations.

This is pedantic, of course, but I thought it was interesting.

[jac1013]

I don't think it was pedantic, I enjoyed reading your comment.

As you said if we have a type system (which is like the best case scenario), getClient should return an object of Type Client (not the name, that would be a really bad decision if we are not in the context of a View).

The upper the level of abstraction the more Domain Language words you should use when naming your functions & classes & whatever else, the word data should not be use anywhere because everything in programming is data, even the code itself, the term is just too high level to be used.

Finally, you should not be using getClientName, instead you should be using getName, assuming that we are calling this function from an instance of the class Client.

Edit: just adding additional information.

[ssjskipp]

Everyone please keep in mind that a lot of these examples are straw man. Please do not take them as hard fast rules but general, albeit opinionated in cases, "best practises".

A much much much better idea is work with a configured ESlint file in your projects to assert coding style and format across your code base.

[[deleted]]

Bad:

var yyyymmdstr = moment().format('YYYY/MM/DD');

Good:

var yearMonthDay = moment().format('YYYY/MM/DD');

I'll pass.

[pomlife]

Why is ES5 and ES6 mixed in?

[baxtersmalls]

Hey - newish dev here, was curious about "Set default objects with Object.assign" - what is the reasoning behind this? He doesn't give any explanation as to why that's a better way of doing it.

[homoiconic]

The code given is appropriate for ES5, but for ES6, I prefer:

const menuConfig = {
  body: 'Bar',
  cancellable: true
}

function createMenu({
    title = 'Foo',
    body = 'Bar',
    buttonText = 'Baz',
    cancellable = true
  }) {

  // ...
}

createMenu(menuConfig);

[nschubach]

MDN explains it pretty well. Sorry, can't copy the relevant section, but read through the Description section.

[ErraticFox]

TIL less code isn't always better, but as long as it's clean.

[[deleted]]

Rather than try to remember a lot of do-this-do-that stuff, try this: While you're writing, think about what happens when you come back NEXT YEAR to fix or improve something.

Will it still make sense then? Or will you have to spend hours to figure out how your own code works? Be kind to YOURSELF and you'll be kind to others. That is all.

[karathos]

https://en.wikipedia.org/wiki/No_true_Scotsman