• @dev_null@lemmy.ml
    link
    fedilink
    66 days 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.

    • @orgrinrt@lemmy.world
      link
      fedilink
      16 days 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.

      • @shape_warrior_t@programming.dev
        link
        fedilink
        English
        16 days 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.

        • @orgrinrt@lemmy.world
          link
          fedilink
          1
          edit-2
          5 days ago

          Yeah obviously with constants for the set roles per value. Some languages call them enums, but the point is that what we pass and use is always still the smallest integer type possible. With the extra bonus that if the roles ever become composable, the same value type would likely suffice for a bitflag and only thing needing refactoring would be bitshifting the constants.

          But anyway, this turns out to be the weirdest hill I find myself willing to die on.

      • @dev_null@lemmy.ml
        link
        fedilink
        1
        edit-2
        6 days 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.

        • @orgrinrt@lemmy.world
          link
          fedilink
          2
          edit-2
          6 days ago

          Yeah, but if it is about being an admin or not, hence the bool, it’d be idiomatic and reasonable to assume it to be false if we have no data. Unless we want to try and allow admin access based on no data. Having three states for a simple binary state is weird. And if it is not about just being an admin or not, the bool is inherently a too limited choice for representation.

          • @dev_null@lemmy.ml
            link
            fedilink
            1
            edit-2
            6 days ago

            Depends on your requirements.

            If the admin status needs to be checked in a database, but most actions don’t require authentication at all, it’s pointless to waste resources checking and it would be left null until the first action that needs the information checks it and fills it in as true or false.

            • @orgrinrt@lemmy.world
              link
              fedilink
              16 days ago

              I don’t really follow you there, wouldn’t it be exactly the opposite and wouldn’t checking for nulls be, as a premise, more wasteful? But doesn’t really matter, time to digress. I’m conventionally educated as an engineer so what I know and find reasonable today might be outdated and too strict for most contemporary stuff.

              • @dev_null@lemmy.ml
                link
                fedilink
                11 day ago

                Let’s say you have a website receiving 1 million requests per day. 0.01% of those are admin requests that need to know if your are an admin to execute them. It would be wasteful to check with the database if you are an admin for every request, when only a tiny minority of then needs to know that. So for 999.900 of the requests isAdmin will be null. We don’t know if the user is an admin and we don’t need to know.

                • @orgrinrt@lemmy.world
                  link
                  fedilink
                  1
                  edit-2
                  1 day ago

                  That all is besides the point. There’s no real advantage to use null instead of defaulting to false there… it’s semantically more accurate and also less wasteful in that other code does not have to worry about nulls which always leads to unnecessary overhead when false is already equivalent in your proposed example.

                  • @dev_null@lemmy.ml
                    link
                    fedilink
                    1
                    edit-2
                    21 hours ago

                    It is not more accurate nor less wasteful. If you default it to false and you have 100 admin requests in the session where that variable is stored, you would have to check with the database 100 times. Because you are conflating false to mean two things: “the user is not an admin” and “we don’t know if the user is an admin”. Where if you set it to true or false the first time we need the information, then subsequent requests don’t need to check anymore.

                    does not have to worry about nulls

                    I am used to null safe languages where there is no such thing as worrying about nulls, because not checking for null on a nullable type is a compile error, and so it’s impossible to forget about the null case. Maybe it’s why I don’t see any issue with using them.