True crime
True crime
True crime
Ah, the ol' tristate boolean switcheroo
Classic checkbox values
Yup. Checked, unchecked, and not checked.
tristate as in three states or tristate as in five states?
Is that a quantum boolean?
That is the jankiest thing I have seen in at least ten years.
Edit: because of course it's office.
i would say why would you just not to isAdmin = true
but i also worked with someone who did just this so i'll instead just sigh.
also the real crime is the use of javascript tbh
And what if it's undefined
?
root access
I see this every sprint.
Sadly this is (or used to be) valid in PHP and it made for some debugging “fun”.
There are several small details that PHP won't allow, but It's valid Javascript and it's the kind of thing you may find on that language.
What if role
is FILE_NOT_FOUND
?!
You could make it even dumber by using weak comparisons.
role is never instantiated, so the... privileged....logs.... will never be called
Edit: Actually no logs at all, I read the null as undefined on first skim
What the fuck
Robert Martin is screaming somewhere. Say what you will about him being out of touch, he did have some good points on writing readable code.
Like null should never be a special value.
And obviously the horrible naming.
Same as ?
C++
std::optional<bool> role; if (role.value()) { std::cerr ("User is admin");} else if (!role.value()) { std::cerr ("User is not admin");} else if (!role.has_value()) { std::cerr ("User is not logged in");}
Here has_value()
should have been checked first, but the JS seems kinda fine.
Which is it?
a === b
returns true if a
and b
have the same type and are considered equal, and false otherwise. If a
is null
and b
is a boolean, it will simply return false.
I see, so logically it is fine.
Just not in the context.
I mean aside of the variable name, this is not entirely unreasonable.
I would certainly rather see this than
{isAdmin: bool; isLoggedIn: bool}
. Withboolean | null
, at least illegal states are unrepresentable... even if the legal states are represented in an... interesting way.Admin false LoggedIn false doesn't feel illegal to me, more redundant if anything
The variable name is 90% why this is so unreasonable. Code is for humans to read, so names matter.
E: omg forget my whole comment. I agree with you that the name sucks.
I mostly don't like that
role
is typically an intuitive name, and now suddenly it means something I wouldn't expect. Why add confusion to your code? I don't always remember what I meant week to week, much less if someone else wrote it.If I had a nickel for every time that happened to me, I’d still be poor, but at least I’d have several nickels. 😁
Product manager: "I want a new role for users that can only do x,y,z"
Developer: "uh.. yeah. About that... Give me a few days."
Hmmm I need a datatype with three states... Should I use a named enum? No, no that's too obvious...
Yeah let’s use a union of a boolean and null to represent role, something that inherently represents more than two (…or three, I guess) different values, as opposed to something like an integer.
Even if the name is clearly misleading in this specific case, the entire choice of using a bool here is just bad because it’s almost guaranteed you’re going to expand on that in future and then you’ll just have to entirely rewrite the logic because it simply can’t accommodate more than two values (or three with the null union… 🙈), while it gives absolute zero benefits over using something more reasonable like an integer to represent the roles, or in this case, admin, not-admin and guest. Even if you’ll end up with just admin, non-admin and guest, the integer would still work great with no disadvantages in terms of amount of code or whatever. Just increased legibility and semantical accuracy.
Not to mention that there’s zero reason to combine the state of being logged in and the role in which you’re logged in in one variable… those are two different things. They will remain two different things in future too…
I mean they’re already chaining elseifs (basically matching/switching, while doing it in an inefficient way to boot 🥴) as though there were an n amount of possible states. Why not just make it make sense from the start instead of whatever the hell this is?
This is quite reasonable, aside from the variable name which should be
isAdmin
. A user either is an admin, or isn't. Unless we don't know, then it's null. You are correct this is bad if the point was to represent roles, but it's not supposed to.