HN Explorer

Poll: The One True Way -- Coding a Conditional

2 points by shawndumas ↗ HN
Help me sleep at night...

Note: JavaScript is the language

See… Because I am an idiot I hate (with a passion) a – d.

I like e; it’s clear and succinct.

I find f very appealing; it’s cool as it makes implicit use of short circuiting – it’s clever and I like clever. But it does not adhere to the adage, ‘Write code like that guy who maintains it is an ax murder that has your address.’

I also like g a lot (again, because there is something wrong with me). But, frankly, it relies on a somewhat esoteric quirk of JavaScript so, again, the ax man cometh…

7 comments

[ 4.3 ms ] story [ 31.8 ms ] thread
Piskvorrr
[dons asbestos longjohns] Let the flamewars begin!

Some of those are outright clever ways of doing the same thing; in source code, that's not a compliment - that's right next to "obfuscation." Source code is not only for the machines, it's also for the unfortunate soul who has to maintain it two years down the line. Note that this might be you, for somebody else's code.

Therefore, the usual maxim applies: "Code as if the next person to maintain your code were an axe-wielding madman who knows where you live."

[deleted]
(comment deleted)
[deleted]
(comment deleted)
mooism2
Your text implies there are seven options (a-g) but there are only five options in the poll.

The fourth option has different behaviour from the first three options should lockBtn.show() return false. Is this what you want? I would expect a comment explaining why if this syntax was used.

The fifth option... I do not think it calls either method (retrieves the function but does not execute it), though my javascript is rusty so I am not sure.

The third option implies that the return value from .show()/.hide() is important and should be used. I would not use it otherwise.

The first two options have their own relative pros and cons, but they are head and shoulders above the other three options.

I do not possess an ax, expect never to be a murderer, and know neither your address nor your appearance. But if I had to maintain code you wrote, and you'd used any of the last three options within it when either of the first two would do, I would slag you off to anyone who would listen.

shawndumas
there were 7. the formatting took multi-line options and made them one line so I just deleted them... while I was removing them the edit option went away. oh-well.

b was the 1st option but on 5 lines; d was the 2nd but on 4 lines.

e is now the 3rd option which is the ternary; and I am unsure of why you are saying that it implies anything about a return value.

in javascript the 4th will evaluate the left side of the and. if it is true it will evaluate the center. if the center is true it will evaluate the right side of the and which will invoke the function. since the function returns a 'truthy' value in javascript the other side of the or does not need to be evaluated because of 'short circuiting'.

if either of the first two parts of the and are false javascript will 'short circuit' the and and move on to the right side of the or and evaluate it which will invoke that function.

you are correct about the last option -- accidentally I left off the invoking parenthesis.

FYI, I like the third option but went with what was b:

    if (data.IsActive && !data.IsLocked) {
        lockBtn.show();
    } else {
        lockBtn.hide();
    }
so all and all it was a complete failure...
mooism2
Option 3/e: it's idiomatic in C-syntax languages to use if/then/else when you do not care about the return value. Use of ?: therefore implies that you do care about the value of the expression. When I see the result of ?: being thrown away, I have to ask myself whether it is supposed to be an if/then/else, or whether a previous programmer meant to do something with the result, but forgot.

Option 4/f: if .show() always returns a truthy value, then yes, this is the same. Even if I know this is true, I must still mentally translate it into the idiomatic form. If I don't know this is true, I must waste time finding out. And if one day .show() doesn't return a truthy value --- perhaps because a different part of the codebase has been changed so that lockBtn is of a different class --- then your code will break.

shawndumas
all true; that's why I went with b.