116
Lightor
7y

Code: if(customer.primaryContract)

Boss: "just using a variable as a boolean isn't very readable"

Me:
if(!(!customer.primaryContract != !true).toString() == 'false')

Comments
  • 0
  • 4
    My tummy hurts
  • 10
    And you let this go to production and the next guy finds it and comes on devRant to rant about it. ;)
  • 3
    There's something to be said for readability by using well chosen names and proper indentation.

    But too often when people use the argument all I hear is "my brain can't handle complexity, please dumb it down" — leading to code which is less readable for those who are competent.
  • 17
    dude, there are good conventions to make code readable.

    if(customer.primaryContract)

    What's that? If customer primary contract what? If customer has one? If the contract is primary? What the fuck does that mean? Boolean values should answer a question:

    customer.hasPrimaryContract
    customer.isPrimaryContract

    Whatever.
    Just answer a fucking question.

    Just like naming the number of contracts like this

    customer.contracts

    Contracts is not a number. It looks like an enumeration of Contract objects.

    Use CONVENTIONS.

    customer.numContracts for example.

    Make your variable names dictate their full meaning, not just the thing they might relate to.

    That's what making code "readable" means.
  • 2
    @nickpapoutsis
    Indeed, if only we could put a tracker on this...
  • 0
    You owe me money for therapy after seeing this :(
  • 2
    @apisarenco this is code to check if his primary contract is defined, yes. That is the convention. The you could throw a != undefined, but that's not even close to needed. As for asking questions, this is in a method called hasPrimaryContract? In the model which is giving you the ability to ask that question. Given the syntax and method name, functionality is very clear.

    Just like .contacts should be a collection of contacts and if you used it in an if statement it would tell you if there were any or if it was an empty/undefined collection. That's how values behave when treated like a Boolean. Having a Boolean stored for all your record values to see if they've been set isn't feasible, you have to be able to make these determinations.
  • 0
    😨
  • 1
    @Lightor well... it's not obvious. Me reading this code and wanting to know what is that member, I'd have absolutely no luck. If I want to manipulate it, should I set it to null? undefined? false? What if it should be null, because in some other place you decided to check it with ===null? Without knowing the entire code base, I can't reliably change this property.

    Treat booleans like booleans and objects like objects. Name them properly and use only non-ambiguous boolean expressions in control statements.
  • 1
    @apisarenco look in something like Angular for example, you check for null/undefined by calling the variable as a Boolean, it's very common convention.

    I get what you're saying but it's a well established convention and practice.
  • 0
    @Lightor this "practice" has landed me in Debugland for days sometimes.
  • 0
    @Lightor you don't call it as a Boolean, it's evaluates to truthy or falsy by coercion and yes, it's very common code to use to check if it has a non-empty property.
Add Comment