• Vilx- (unregistered)

    Bet it was an inTERN who wrote this TERRNible code.

  • bvs23bkv33 (unregistered)

    if Captain Jack Sparrow is the best pirate I have ever seen, then Will Terner will be the best TERNer I have ever seen

  • King (unregistered)

    Did you find this on the inTERNet?

  • someone (unregistered)

    Seeing a bad trend recently – Remy starts every article talking about himself.

  • Vilx- (unregistered)

    At least there was an alTERNative.

  • (nodebb)

    Sadly, the replacement function doesn't account for the fact that JS has falsy values that are neither null nor undefined, and the behaviour is changed for those values.

  • my name is missing (unregistered)

    Tern away from javascript, lest ye lose all hope,

  • Jonathan (unregistered)

    Another WTF was missed, the poor method name which breaks encapsulation.

    The method name in my mind implies that it is a "command" that will disable the "estimateSent" when called, but in actual fact it's "query" whose return value lets you know whether or not estimateSent should be enabled.

    I would rather name it something like one of the following:

    • "isEstimateSentEnabled"
    • "estimateSentIsEnabled"
    • "estimateSent" (would probably be my choice in an MVVM scenario, and then have something like enabled="{{ !estimateSent }}" in my view.)
  • bvs23bkv33 (unregistered) in reply to someone

    bad ternd

  • RLB (unregistered) in reply to Steve_The_Cynic

    Sadly, the replacement function doesn't account for the fact that JS has falsy values that are neither null nor undefined,

    Not, unless I'm missing something, if that object is a boolean.

  • (nodebb)

    Sadly, the replacement function doesn't account for the fact that JS has falsy values that are neither null nor undefined,

    Not, unless I'm missing something, if that object is a boolean.

    If the value is the integer 0, the old function returns !true ==> false, but the new function returns !(falsy==>false) ==> true.

    And of course I wrote my initial comment wrong. It should have been "neither false nor undefined". null is falsy, but its return, too, would change from false in the old to true in the new because it isn't false and it isn't undefined. (Unlessly unless I missed something about null == false and/or null == undefined

    Oh, bugger. It's JS, so that == is going to do reckessly heroic (heroically reckless?) levels of conversion. Ugh. Never mind.

  • (nodebb)

    Wtf? The biggest wtf is mixing 4 different seemingly unrelated concepts: "estimate," "survey," disable/enable, send/receive

  • FormalWare (unregistered)

    return !modalControl.survey[0].Survey_Complete

    That's danged elegant - especially when compared to the monstrosity it replaced. Calling it "prosaic" minimizes the value of clean code.

  • Mobile user (unregistered)

    The new function is not equivalent to the old one. It should be !(modalControl.survey[0].Survey_Complete == false) in order to work the same way. The original only returns true if the variable is false (not falsy), if I'm not mistaken.

  • Mobile user (unregistered) in reply to Mobile user

    Welp, I misread the code, so nevermind. Steve's point still stands.

  • jgh (unregistered)

    falsy? Surely falsey? aaiiii!!! falsies!

  • JO (unregistered)

    I'm throwing my un-ternarying change before the judges (for me it was a huge improvement in read-/understandability):

    Before: return labelKey == null ? null : varyingToolTipText ? messageSource.getMessage(new DefaultMessageSourceResolvable(new String[] {labelKey + TOOLTIP_SUFFIX, labelKey})) : shortNameInList ? messageSource.getMessage(labelKey) : null;

    After: if (toolTipMessageKey != null) { return messageSource.getFormattedMessage(toolTipMessageKey, toolTipArguments); }

    	if (labelKey == null) {
		return null;
	}

	if (varyingToolTipText) {
		return messageSource.getMessage(new DefaultMessageSourceResolvable(new String[] { labelKey + TOOLTIP_SUFFIX, labelKey }));
	}

	if (shortNameInList) {
		return messageSource.getMessage(labelKey);
	}

	return null;
  • Doug (unregistered) in reply to JO

    Is this a "it's all in the formatting" thing? (I see the BB has helpfully munted some of your formatting -- we'll see how my reply fares.) If you formatted it like the below, would that be equally clear?

    
return 

    labelKey == null ? 

        null :

    varyingToolTipText ? 

        messageSource.getMessage(new DefaultMessageSourceResolvable(new String[] {labelKey + TOOLTIP_SUFFIX, labelKey})) : 

    shortNameInList ? 

        messageSource.getMessage(labelKey) : 

    null;
  • (nodebb)

    It is not a bad idea to avoid refraining from not using multiple negating cascades.

  • (nodebb) in reply to RLB

    0==false true 0===false false process.version 'v8.12.0'

  • a_dinosaur (unregistered)

    Remy, your code should be:

    const ControllerState response = allMotorsIdle() ? READY : NOT_READY;

    The ability to initialize a constant with the const keyword is IMHO a much overlooked advantage of the ternary operator.

  • Andrew (unregistered)

    Meh. This is not even the worst code that I have seen today.

  • Nobody (unregistered)

    That's not even bad. Clearly the ternary is wrong but by the time you've seen the "return !" you can mentally switch true/false in the prevkious line and, because a ternary returns a boolean, it immediately cancels itself out and the whole function collapses into a simple "double test". If I was refactoring I'd make that change then and there before even thinking about what the function did. If I was just reading code I'd make a mental note and just carry on.

    I give this one 4 out of 10.

Leave a comment on “Tern The Bool Around”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article