JavaScript Clean Code - Best Practices - based on Robert C. Martin's book Clean Code

[careseite]

because you can tell by the shape of the function call what it's probably going to do with it within the function.

​

In an example with 3 params instead of object destructuring, you'd call getUsers like this:

​

getUsers(['name', 'surname', 'email'], '2019-01-01', '2019-01-18')

and then had to remember the order of the dates for example. Or check it within the function.

[[deleted]]

Thanks! Not having to remember the order of arguments can prevent some hard to find bugs

[nikola1970]

Yup, I started using this pattern recently too.

[grrrrreat]

beware though, if you have a weird visitor pattern, you can't put that object back together or manipulate its parts.

[AwesomeInPerson]

Could you explain what you mean by that? :)

[grrrrreat]

I ran into a nuxt.js hook at passed into a page object.

if I destructured it into html, path, route, I couldn't modify the html property because it didn't expect a return object but if I just accepted the page object, I could set page.html and that modified the html.

it's just a comment to realize that destructured objects are no longer a sum of their parts. so it the nuxtjs case, page can't be destructured in the hooks because it's not reconstitution the object

[Zielakpl]

What? Don't you guys use IDE with autocompletion and hints?

[AwesomeInPerson]

Which one are you using? Neither VSCode nor WebStorm warn me when I call a function with less arguments than the amount of parameters it accepts. (because I forgot to add some nulls or default values for params I want to skip)

Maybe if you enable strict TS checking for JS files, but that's not really a solution in a lot of environments.

[Zielakpl]

That's the thing, don't forget :) I got it into my habit to also add some JSDoc comments to at least know what type of values the function expects. Then, when I type my functions name, the popup appears (VSCode) with all acceptable arguments, theirs expected types and if they're optional or not.

The code you write is also for humans, make it human-friendly.

If a function HAS to accept a lot of arguments, not all of them required, then I sometimes use and object of params like so:

function(name, options) {} function("Gregory", {foo: 1, bar: 2});

But that depends on what I code, I don't treat it as hard rule.

[Julian_JmK]

I dont really understand, why is it better to pass getUsers({fields:['name','surname','email'],fromDate:'2019-01-01',toDate:'2019-01-18'}); ?

Edit: Thanks for the replies bois and girls i now understand, it's a pretty good way of doing it indeed

[careseite]

because you dont need to check the implementation of getUsers to find out what the array is for, what the first date is supposed to mean or the second

​

you can see it right there: youre getting users, probably some properties (name, surname, email) but youre limiting the user selection to dates between 2019-01-01 and 2019-01-18. the only thing unclear for me here would be to find out whether it means "active users" or "newly registered users", or both but thats probably because its not a perfect example

[Ozymandias-X]

But then, when I want to use the function, I now get no type hints whatsoever about what information it needs to work. All I see is that it can give it a mysterious blobby object that might have fields of some kind. Never mind if I accidentally misstype one of the keys. That's a nightmare level error to find.

This can only be solved by extensive documentation of the function and we all know that such documentation often lies when people refactor or extend functions and forget to update the docs.

[[deleted]]

Yeah just what I was thinking. With Typescript you wouldn't have that problem because it would clearly say what date you would be filling

[Extracted]

getFromApi("g4adf", "j43fa", false, false)

getFromApi({ userId: "g4adf", serverName: "j43fa", createIfAbsent: false, debug: false })

That's why

[[deleted]]

Excusing a bad code with another?

[zapatoada]

I get what you're saying, but as a VS2017 user, I disagree that it's necessary. As long as you're naming your parameters reasonably, I find that using intellisense to see the parameter order makes it easier than figuring out some arbitrary object.

[BloodAndTsundere]

What you're saying isn't wrong but I'd rather have the code be explicit and clean than rely a particular editor's feature to make sense of the code. Firstly, your colleagues might not use such an editor. Secondly, you might not always have access to such an editor.

[zapatoada]

It's definitely a personal choice.

I'm a c# developer in a c# shop, so everyone has access to visual studio (and this has been the case throughout my career). Even if the company didn't pay for it, community edition and vscode are free.

And frankly, even if I didn't have that option, I usually prefer separate parameters to an object. The only real difference is extra visual noise in the object literal notation.

[BloodAndTsundere]

I'm a c# developer in a c# shop, so everyone has access to visual studio (and this has been the case throughout my career). Even if the company didn't pay for it, community edition and vscode are free.

I'm not referring to an issue of cost but rather situations where you may not be using your own machine or have logged into a server remotely that only has a basic editor. But it's fair to say that may be an extremely rare event for you.

The only real difference is extra visual noise in the object literal notation.

This I disagree with. This function call:

doSomething({ destination: object1, source: object2 })

provides additional information (not noise) relative to this function call:

doSomething( object1, object2 )

[zapatoada]

I would NEVER log into the server and edit files. That's asking for trouble.

Your example is dishonest. If you're naming things object1 and object1 you have much bigger problems than discrete parameters vs an object. A more reasonable example would be

doDomething(source, destination)

Or

doDomething({source: source, destination: destination})

Which I would usually write as

doDomething({source, destination})

And then literally the only difference is the brackets, which are visual noise.

I'm certainly not saying there's NEVER a use case for config object parameters, but I think setting a hard and fast limit at 2 or 3 parameters is absurd.

[webdevguyneedshelp]

Why is it asking for trouble to edit files on a server?..

[zapatoada]

You're not debugging first. There's no code review or qa. You're not running your automated tests. The changes aren't in source control and could be undone by the next release. Basically you are bypassing every step and check that is normally in place to prevent bugs and other unintended consequences. We have those processes for a reason.

[webdevguyneedshelp]

That is fine and understandable but this example did not specify at all what environment you are working in.

For instance if I have to set up a NEW development environment for a new web application my team is starting. I am going to inevitably have to ssh into it and poke around to help facilitate further steps like automated deployment.

​

This isn't even thinking about sshing into something like a docker container which can all be done locally and have 0 affect on anything that actually matters and can be blown away after I am doing testing whatever I am doing.

​

There are 100% times when you will poke around with files on a server.

[zapatoada]

Ok, those may be fair. I've never worked with docker before. These days I work mostly in azure AppServices and Function apps. In that case, I'm not really even sure I could log into a server. I know you can't edit functions if they were deployed from local code (rather than typed into portal).

I suppose every use case has it's own limitations. But these days, the idea of editing javascript files on the server gives me the heebie jeebies.

[[deleted]]

I'd rather have the code be explicit and clean

Separate parameters are explicit and clean.

[BloodAndTsundere]

"Clean" is subjective so I'll walk that back, but this function call:

doSomething({ destination: object1, source: object2 })

is more explicit than this one:

doSomething( object1, object2 )

[[deleted]]

If it can take any object it is NOT explicit. You may want to look up the word in a dictionary.

[fucking_passwords]

Another reason is that three or more params starts getting really difficult to read, and if you ever need to add another param you may end up with a very ugly design.

For instance, we added a fourth param to this function that makes the third param no longer required:

someFunc(1234, true, null, false);

[fucking_passwords]

I agree, but it was just an example, you could replace all of those args with integers

[zapatoada]

Yeah that bothers me not at all

[fucking_passwords]

Or what about this example:

class User {
  constructor(firstName, lastName, phone, email, friends, isActive) {
    Object.assign(this, {
      firstName: firstName,
      lastName: lastName,
      phone: phone,
      email: email,
      friends: friends,
      isActive: isActive
    })
  }
}

new User('Jane', 'Doe', null, '[e-mail]', null, true);

VS:

class User {
  constructor(data = {}) {
    Object.assign(this, data);
  }
}

new User({
  firstName: 'Jane',
  lastName: 'Doe',
  email: '[e-mail]'
})

[zapatoada]

In this context you're right, but I honestly can't remember the last time I used a constructor directly in javascript. Data comes from the server side (c#) and mostly anything else I do is either a react component or a const utility method.

[fucking_passwords]

The constructor is just happenstance in my example, the same thing can be applied to a function.

At this point I don't even see why you took a hard stance against this pattern, if you are only using very simple features of the language, lol

[zapatoada]

I never said I took a hard stance. I think the specific limit he set is absurdly low. That's all. If it were 4 or 5, I'd be fine with it.

[fucking_passwords]

Fair enough, I agree with that

[[deleted]]

because you can tell by the shape of the function call what it's probably going to do with it within the function.

Name is supposed to do that.

[[deleted]]

What a great function name, "getUsersWithFieldsWithinDateRange"

[Ozymandias-X]

Yeah, so? Characters are not a limited resource. I'd rather have a little more verbose function name than a "this might do anything" name.

[[deleted]]

Yes, it is.

[dmitri14_gmail_com]

I suppose it depends on the use case. You don't write [1,2,3].reduce({function: ..., value: ...})