You are viewing a single thread.
View all comments
67 points

Ew no.

Abusing language features like this (boolean expression short circuit) just makes it harder for other people to come and maintain your code.

The function does have opportunity for improvement by checking one thing at a time. This flattens the ifs and changes them into proper sentry clauses. It also opens the door to encapsulating their logic and refactoring this function into a proper validator that can return all the reasons a user is invalid.

Good code is not “elegant” code. It’s code that is simple and unsurprising and can be easily understood by a hungover fresh graduate new hire.

permalink
report
reply
47 points

Agreed. OP was doing well until they replaced the if statements with ‚function call || throw error’. That’s still an if statement, but obfuscated.

permalink
report
parent
reply
7 points

Don’t mind the || but I do agree if you’re validating an input you’d best find all issues at once instead of “first rule wins”.

permalink
report
parent
reply
3 points

Short circuiting conditions is important. Mainly for things such as:

if(Object != Null && Object.HasThing) …

Without short circuit evaluation you end up with a null pointer exception.

permalink
report
parent
reply
20 points

I agree, this is an anti-pattern for me.

Having explicit throw keywords is much more readable compared to hiding flow-control into helper functions.

permalink
report
parent
reply
14 points

This is the most important thing I’ve learned since the start of my career. All those “clever” tricks literally just serve to make the author feel clever at the expense of clarity and long-term manintainability.

permalink
report
parent
reply
8 points

100% un-nesting that if would have been fine.

permalink
report
parent
reply
6 points

I mean, boolean short circuit is a super idiomatic pattern in Javascript

permalink
report
parent
reply
7 points

I think that’s very team/project dependent. I’ve seen it done before indeed, but I’ve never been on a team where it was considered idiomatic.

permalink
report
parent
reply
1 point

That makes sense.

permalink
report
parent
reply
2 points

Because on JS the goal is to shave bytes to save money on data transfer rates

permalink
report
parent
reply
2 points

It’s not that deep. It looks nice, and is easy to understand.

permalink
report
parent
reply
5 points
*

Good code is not “elegant” code. It’s code that is simple and unsurprising and can be easily understood by a hungover fresh graduate new hire.

I wouldnt go that far, both elegance are simplicity are important. Sure using obvious and well known language feaures is a plus, but give me three lines that solve the problem as a graph search over 200 lines of object oriented boilerplate any day. Like most things it’s a trade-off, going too far in either direction is bad.

permalink
report
parent
reply
1 point
*

assert(isPasswordGood(…)) is already in the language. node

permalink
report
parent
reply

Programming

!programming@programming.dev

Create post

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person’s post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you’re posting long videos try to add in some form of tldr for those who don’t want to watch videos

Wormhole

Follow the wormhole through a path of communities !webdev@programming.dev



Community stats

  • 3.1K

    Monthly active users

  • 891

    Posts

  • 7.7K

    Comments