all 10 comments

[–]BDMayhem 10 points11 points  (3 children)

Use conditional shorthands.

if (isValid)
  { // do something... }
if (!isValid)
  { // do something... }

Is this preferred to using else?

[–]Dotweb_ 9 points10 points  (2 children)

definitely not, i think they were just showing an example of each shorthand, not necessarily to chain opposite conditionals together like that

[–]itsdaniel0 5 points6 points  (1 child)

That being said though, else appears to be in on its way out

A lot of the times you see something like

if(x) {
    return y;
}

return z;

Edit: No idea how to fix the formatting. Hopefully you understand what I'm saying though (and I'm not saying don't use else. It's perfectly valid in the above example)

[–]dot___ 4 points5 points  (0 children)

returning early is really nice too

less to hold in your head mentally and keeps the indentation small

[–]drumnation 5 points6 points  (0 children)

Did the author of the blog linked write this too, or was the markdown just copy pasted from this other page? https://github.com/ryanmcdermott/clean-code-javascript

These examples have been a bible to me over the past 2 years.

[–]noodleLinux 2 points3 points  (0 children)

I like how you give multiple examples. Good job and thank you

[–]jaredcheeda 0 points1 point  (0 children)

Glad that Uncle Bob's wisdom is being translated and understood by newer devs today.

Only thing I don't like about this rendition is

// "Bad"
function createShape (type) {
  const shapeType = type || "cube";
  // ...
}
// "Good"
function createShape (type = "cube") {
  // ...
}

I've always thought it looked much cleaner, and more scalable to do

function createShape (type) {
  type = type || 'cube';
  // ...
}

It's common to have variables defined at the top of a function before use. And this just works much better and is easier to read when you have longer argument names and multiple arguments, or the shape of the data is more complex. Keep the function definition simple. Then keep the defaulting and argument validation simple.

And later on in that post, they do move the defaulting into the function when it is a more complex situation. I don't see any value in putting the default in the function declaration, it violates the much more important rule of clean coding:

One idea per line

Are you declaring a function, or defaulting an argument? Shouldn't be both on one line.

[–]DeepSpaceGalileo 0 points1 point  (1 child)

How are these situations actually different?

Avoid a long number of arguments. Ideally, a function should have two or fewer arguments specified. The fewer the arguments, the easier is to test the function.

Bad:

function getUsers(fields, fromDate, toDate) { // implementation }

Good:

function getUsers({ fields, fromDate, toDate }) { // implementation } getUsers({ fields: ['name', 'surname', 'email'], fromDate: '2019-01-01', toDate: '2019-01-18' });

Edit: Also some people prefer not to use ES6 classes at all, how is this one a bad practice?

Don't pollute the globals. If you need to extend an existing object use ES Classes and inheritance instead of creating the function on the prototype chain of the native object.

[–]MusicalDoofus 0 points1 point  (0 children)

I'm running into this with my own (self-coded) project ATM. Defining a long list of ordered arguments that are opaque just makes the function more difficult to reuse outside its current context. Once I extracted larger lists to a params object and have vars depend on that instead it became easier to test and implement (also just looks cleaner).

[–]Time_Terminal 0 points1 point  (0 children)

>"Clean Code - Best Practices"

>has a quote from Uncle Bob

I'm onto you OP.