Apparently `a || b` is unreadable to set a default value for a string in JS, but assigning temporary variables and use if statements all over is better somehow.

My code reviewer said use `a ? a : b` except it failed eslint, so he went back on that. (eslint suggested `a || b`, ffs.)

do you have some kind of convention of what to use? we update our eslintrc every time we agree what is better way to write something. so our convention is forced trough eslint.

And yeah a||b is totally valid, we also use a&&b instead of a?b:null, also we use things like !!variable, short if statements without braces at all, short returns, arrow functions without braces around single argument or curlys if we return something etc.

`a || b` is the clearest approach for individual vars. Ternaries are a horrible choice here.

Merging custom values over defaults (or better yet: ES6 destructuring) is also very readable, and preferable for larger sets of defaults. (it's not as performant, however, but the difference is negligible for most usecases).

OP, your reviewer is a tool.

@legacyJanitor

Im a bit confused... a&&b and a?b:null do different things, can i see an example?

@legacyJanitor

I'm just a co-op so i don't have much say here.

Some other gems from our styleguide: no semicolons (semicolons are an error), always put braces around if statement, no ternary in most cases (prefer if-else).

@D--M not the perfect one but from top of my head

const {error} = this.state
const msg = 'error msg'
<component
isDisabled={error && msg} />

so if error = true, give me msg, if not it doesnt go further and it is kinda same as error ? msg : null

a || b is not unreadable. a && b is not unreadable either.

@legacyJanitor I reckon you are not so familiar with Javascript. Its not the same. In your first case, it goes further and assigns undefined to isDisabled variable, if error doesnt exist. I would except a boolean from isDisabled and i don't wanna see a message there, and not an undefined either. Javascript is not a strictly typed language, so your variable naming has more importance.

Your second case is completely different.

error ? msg : null;

is not gonna do anything. A similar thing :

isDisabled = error ? msg : null;

Will assign msg to isDisabled if error exists. If not, your isDisabled will be null. Never assign to null unless undefined won't cut it.

@illegaldisease
In terms of actual readability a && b is not readable. It requires the reader to intricately understand how boolean operators work and its not intuitive.
a || b on the other hand works exactly as it reads and its intuitive, so thats ok.
I would rather replace a ? b : null instead of a && b. Since it reads way better and its easier to understand right of the gate.
You are working in a team and most of the time unfortunately not everyone is on the same level of knowledge there, so this is a better compromise for everybody and it doesn't cost much to write.
I also make the similar mistake with shortening checked function calls. Like this:
if (a) {
somefunction();
}
Into:
a && someFunction();
But this is also not very readable, even though it works exactly as it reads. 😊

@illegaldisease about undefined, yes and no. error is false until it happens, then toggles to true. we use flow so error: boolean, false by default.

so it can return either msg or false, maybe idk JS, but that seems fine to me as no stone left undefined.

Also about readability, we have daily code reviews and every time something new needs to be introduced, we have a explaining/learning session, so we all know how and why. We are small team, so no big issues about understanding whats going on. Also when someone new shows up, we have introduction/individual tutoring to all things we use (also how and why). I worked in much bigger company before and I know what you are talking about (everyone pushes its own preferences, no strict conventions and not all on the same page), so i agree.

@arekxv - that last one, no dude, we would never use that! actually we use it only inside react render method on components where needed, something like I showed above.

Also no nulls - I just used it in example, I am aware that is not good practice.

I realised how shitty my example was, you were right, isDisabled is completely wrong in logic and in naming, it should be errorMessage. that makes sense, my current example above does not!

@legacyJanitor you are completely right on every dev pushing their own references, maybe i do the same too, sorry about that. And yeah, the most important thing is to learn from our mistakes.

@arekxv i explained exactly why you shouldn't use null like that. At least use undefined or have some default value. To me, as long as it has a decent name, i want the snippet as short as it can get.