dohpaz42 83 points a year ago

I mean aside of the variable name, this is not entirely unreasonable.

path: 0 18801213, hotness: undefined, score: 83, children: 25

shape_warrior_t 34 points a year ago

I would certainly rather see this than {isAdmin: bool; isLoggedIn: bool}. With boolean | null, at least illegal states are unrepresentable... even if the legal states are represented in an... interesting way.

save

path: 0 18801213 18803188, hotness: undefined, score: 34, children: 4

nialv7 3 points a year ago

Admin false LoggedIn false doesn't feel illegal to me, more redundant if anything

save

path: 0 18801213 18803188 18812369, hotness: undefined, score: 3, children: 3

shape_warrior_t 9 points a year ago

I was thinking of the three legal states as:

not logged in (null or {isAdmin: false, isLoggedIn: false})
logged in as non-admin (false or {isAdmin: false, isLoggedIn: true})
logged in as admin (true or {isAdmin: true, isLoggedIn: true})

which leaves {isAdmin: true, isLoggedIn: false} as an invalid, nonsensical state. (How would you know the user's an admin if they're not logged in?) Of course, in a different context, all four states could potentially be distinctly meaningful.

save

path: 0 18801213 18803188 18812369 18813225, hotness: undefined, score: 9, children: 2

nialv7 4 points a year ago

ah you are right! i am so dumb.

save

path: 0 18801213 18803188 18812369 18813225 18815886, hotness: undefined, score: 4, children: 0

chocrates 1 point a year ago

Honestly logged in is state and shouldn't be on the user object.

save

path: 0 18801213 18803188 18812369 18813225 18817088, hotness: undefined, score: 1, children: 0

Drewmeister 8 points a year ago

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.

save

path: 0 18801213 18801938, hotness: undefined, score: 8, children: 1

dohpaz42 7 points a year ago

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. 😁

save

path: 0 18801213 18801938 18802308, hotness: undefined, score: 7, children: 0

grrgyle 7 points a year ago

The variable name is 90% why this is so unreasonable. Code is for humans to read, so names matter.

save

path: 0 18801213 18810949, hotness: undefined, score: 7, children: 0

normalexit 3 points a year ago

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."

save

path: 0 18801213 18805172, hotness: undefined, score: 3, children: 1

grrgyle 5 points a year ago

Hmmm I need a datatype with three states... Should I use a named enum? No, no that's too obvious...

save

path: 0 18801213 18805172 18810963, hotness: undefined, score: 5, children: 0

orgrinrt -1 points a year ago

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?

save

path: 0 18801213 18806596, hotness: undefined, score: -1, children: 14

dev_null 7 points a year ago

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.

save

path: 0 18801213 18806596 18807091, hotness: undefined, score: 7, children: 13

orgrinrt 1 point a year ago

Admin is a role though, was my point. And besides, if you check for three different states, and you decide to go with a boolean to represent that, I really find it hard to believe anyone would think it reasonable. It’s valid and it’s practical, but can you really say it’s reasonable?

I don’t do typescript, but wouldn’t a union of a null and a bool be just more resource intensive than simply using an unsigned byte-sized integer? I struggle to find reasons to ever go for that over something more reasonable and appropriate for what it attempts to represent (3 distinct states as it stands, and likely in future more than just 3 when they have a need for more granularity, as you’d often do with anything you’d need an admin role distinction in the first place), but likely I’m just not familiar with ts conventions. Happy to hear the reasoning for this though.

save

path: 0 18801213 18806596 18807091 18808351, hotness: undefined, score: 1, children: 12

dev_null 1 point a year ago

So in a language with nullable types, are you against a boolean ever being nullable? Null means "empty, missing info". Let's say we have role variable with a enum type of possible roles. It could still reasonably be nullable, because in some scenarios you don't know the role yet, like before log in.

In any use case where we need to store some boolean, it's a common occurrence that we don't have the data and it's null. It would be overkill to use an enum with True, False, NoData for these cases, where there is already a language feature made just for that, nullable values.

I've never used TypeScript, just writing from experience in other languages.

save

path: 0 18801213 18806596 18807091 18808351 18808438, hotness: undefined, score: 1, children: 9

shape_warrior_t 1 point a year ago

My preferred way of modelling this would probably be something like
role: "admin" | "regular" | "logged-out"
or
type Role = "admin" | "regular";
role: Role | null
depending on whether being logged out is a state on the same level as being a logged-in (non-)admin. In a language like Rust,
enum Role {Admin, Regular}
instead of just using strings.

I wouldn't consider performance here unless it clearly mattered, certainly not enough to use
role: number,
which is just about the least type-safe solution possible. Perhaps
role: typeof ADMIN | typeof REGULAR | typeof LOGGED_OUT
with appropriately defined constants might be okay, though.

Disclaimer: neither a professional programmer nor someone who regularly writes TypeScript as of now.

save

path: 0 18801213 18806596 18807091 18808351 18812481, hotness: undefined, score: 1, children: 1

bhamlin 57 points a year ago

Ah, the ol' tristate boolean switcheroo

save

path: 0 18801492, hotness: undefined, score: 57, children: 5

kionay 4 points a year ago

tristate as in three states or tristate as in five states?

save

path: 0 18801492 18819490, hotness: undefined, score: 4, children: 2

perviouslyiner 3 points a year ago

Is that a quantum boolean?

save

path: 0 18801492 18819490 18820192, hotness: undefined, score: 3, children: 0

bhamlin 2 points a year ago

That is the jankiest thing I have seen in at least ten years.

Edit: because of course it's office.

save

path: 0 18801492 18819490 18822104, hotness: undefined, score: 2, children: 0

notarobot 2 points a year ago

Classic checkbox values

save

path: 0 18801492 18835611, hotness: undefined, score: 2, children: 1

bhamlin 1 point a year ago

Yup. Checked, unchecked, and not checked.

save

path: 0 18801492 18835611 18835840, hotness: undefined, score: 1, children: 0

cupcakezealot 30 points a year ago

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

save

path: 0 18801252, hotness: undefined, score: 30, children: 15

Lemminary 23 points a year ago

That's TypeScript. I can tell by the pixels defining a type above.

save

path: 0 18801252 18801371, hotness: undefined, score: 23, children: 14

Maiq 6 points a year ago

Was looking at it and could not figure out why their weren't any semicolon's.

save

path: 0 18801252 18801371 18803066, hotness: undefined, score: 6, children: 13

ScintillatingStruthio 13 points a year ago

Neither Javascript nor Typescript require semicolon, it is entirely a stylistic choice except in very rare circumstances that do not come up in normal code.

save

path: 0 18801252 18801371 18803066 18803965, hotness: undefined, score: 13, children: 12

Lemminary 13 points a year ago

Explanation for nerds

The reason is the JS compiler removes whitespace and introduces semicolons only "where necessary".

So writing

function myFn() {
  return true;
}

Is not the same as

function myFn() {
  return 
    true;
}

Because the compiler will see that and make it:

function myFn() { return; true; }

You big ol' nerd. Tee-hee.

save

path: 0 18801252 18801371 18803066 18803965 18804443, hotness: undefined, score: 13, children: 3

Maiq 4 points a year ago

That's good to know. Don't know how I didn't know this. Been writing JS since 2000. Always just used them I guess. Ecmascripts look funny to me without them

save

path: 0 18801252 18801371 18803066 18803965 18804200, hotness: undefined, score: 4, children: 4

Ephera 2 points a year ago

Hmm, a webdev colleague said he'd normally prefer without semicolons, but used them anyways for better compile errors.

save

path: 0 18801252 18801371 18803066 18803965 18804904, hotness: undefined, score: 2, children: 2

tdawg 27 points a year ago

This is pretty clearly just rage bait. Nothing is actually setting the value so it's undef. Moreover there isn't any context here to suggest if the state definitions are determined by some weird api or are actually just made up

save

path: 0 18801426, hotness: undefined, score: 27, children: 4

victorz 8 points a year ago

Troof

I mean facts. Facts is what the kids say. Facts.

save

path: 0 18801426 18801804, hotness: undefined, score: 8, children: 3

user224 5 points a year ago

*Fax

save

path: 0 18801426 18801804 18802127, hotness: undefined, score: 5, children: 2

obinice 5 points a year ago

We don't use fax machines any more grandad! It's all twoggles now! Twoggle me a nurp!

save

path: 0 18801426 18801804 18802127 18803682, hotness: undefined, score: 5, children: 0

victorz 3 points a year ago

Dammit!

save

path: 0 18801426 18801804 18802127 18808695, hotness: undefined, score: 3, children: 0

livingcoder 14 points a year ago

I see this every sprint.

save

path: 0 18806565, hotness: undefined, score: 14, children: 0

ramble81 13 points a year ago

Sadly this is (or used to be) valid in PHP and it made for some debugging “fun”.

save

path: 0 18803991, hotness: undefined, score: 13, children: 1

marcos 9 points a year ago

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.

save

path: 0 18803991 18804407, hotness: undefined, score: 9, children: 0

jbrains 10 points a year ago

What if role is FILE_NOT_FOUND?!

save

path: 0 18801281, hotness: undefined, score: 10, children: 8

foxglove 15 points a year ago

if it's 'FILE_NOT_FOUND' then the string will be read as truthy and you will get 'User is admin' logged.

save

path: 0 18801281 18801572, hotness: undefined, score: 15, children: 7

bjoern_tantau 30 points a year ago

Ackshually three equal signs check for type as well. So mere truthiness is not enough. It has to be exactly true.

Also, everyone knows FILE_NOT_FOUND isn't a string but a boolean value.

save

path: 0 18801281 18801572 18801782, hotness: undefined, score: 30, children: 6

foxglove 1 point a year ago

yeah, it's funny how my brain collapsed the boolean check into if (role) rather than if (role === true) - that's tricky

what is FILE_NOT_FOUND? I can't find much on it ...

save

path: 0 18801281 18801572 18801782 18816266, hotness: undefined, score: 1, children: 5

bjoern_tantau 5 points a year ago

FILE_NOT_FOUND is from an old story on thedailywtf.com. Someone created a boolean enum with TRUE, FALSE and FILE_NOT_FOUND, if I recall correctly. It's been a recurring running joke.

save

path: 0 18801281 18801572 18801782 18816266 18816267, hotness: undefined, score: 5, children: 4

9point6 7 points a year ago

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

save

path: 0 18801378, hotness: undefined, score: 7, children: 0

monkeyslikebananas2 6 points a year ago

What the fuck

save

path: 0 18801187, hotness: undefined, score: 6, children: 0

BigBenis 4 points a year ago

And what if it's undefined?

save

path: 0 18820863, hotness: undefined, score: 4, children: 1

tfm 9 points a year ago

root access

save

path: 0 18820863 18825218, hotness: undefined, score: 9, children: 0

bitjunkie 3 points a year ago

You could make it even dumber by using weak comparisons.

save

path: 0 18813256, hotness: undefined, score: 3, children: 0

ulterno 1 point a year ago

Same as ?

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?

save

path: 0 18808615, hotness: undefined, score: 1, children: 2

shape_warrior_t 4 points a year ago

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.

save

path: 0 18808615 18812082, hotness: undefined, score: 4, children: 1

ulterno -1 points a year ago

I see, so logically it is fine.
Just not in the context.

save

path: 0 18808615 18812082 18817348, hotness: undefined, score: -1, children: 0

grrgyle 1 point a year ago

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.

save

path: 0 18819904, hotness: undefined, score: 1, children: 0

Rules

Rules