• shape_warrior_t@programming.dev
      link
      fedilink
      English
      arrow-up
      32
      ·
      3 months 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.

        • shape_warrior_t@programming.dev
          link
          fedilink
          English
          arrow-up
          9
          ·
          3 months 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.

    • grrgyle@slrpnk.net
      link
      fedilink
      arrow-up
      8
      ·
      3 months ago

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

    • Drewmeister@lemmy.world
      link
      fedilink
      arrow-up
      8
      ·
      3 months 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.

      • dohpaz42@lemmy.world
        link
        fedilink
        English
        arrow-up
        7
        ·
        3 months 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. 😁

    • normalexit@lemmy.world
      link
      fedilink
      arrow-up
      3
      ·
      3 months 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.”

      • grrgyle@slrpnk.net
        link
        fedilink
        arrow-up
        5
        ·
        3 months ago

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

    • orgrinrt@lemmy.world
      link
      fedilink
      arrow-up
      1
      arrow-down
      2
      ·
      3 months 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?

  • katy ✨@piefed.blahaj.zone
    link
    fedilink
    English
    arrow-up
    30
    arrow-down
    3
    ·
    3 months 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

      • Maiq@lemy.lol
        link
        fedilink
        arrow-up
        6
        arrow-down
        1
        ·
        3 months ago

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

        • ScintillatingStruthio@programming.dev
          link
          fedilink
          English
          arrow-up
          12
          ·
          3 months 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.

          • Lemminary@lemmy.world
            link
            fedilink
            arrow-up
            12
            ·
            3 months 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.

              • Lemminary@lemmy.world
                link
                fedilink
                arrow-up
                1
                ·
                3 months ago

                Not wrong, but funnily enough, it’s a linting rule win. I’d go nuts if I didn’t have my type checks and my linters. My current L, though, is setting up the projects initially and dealing with the configuration files if I raw dog it, but that’s a problem with ESLint configs and the ecosystem as a whole having to deal with those headaches. So in the end, the JS devs got clever and shifted the blame to the tooling. 😅

          • Maiq@lemy.lol
            link
            fedilink
            arrow-up
            4
            ·
            3 months 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

            • ScintillatingStruthio@programming.dev
              link
              fedilink
              English
              arrow-up
              2
              ·
              3 months ago

              Fair enough, I like it better without but I don’t have a strong preference and have no issue adapting to whatever the style of the repo is.

              I learned about it researching tools to automatically enforce formatting style and came across StandardJS, which eliminates them by default.

              • Maiq@lemy.lol
                link
                fedilink
                arrow-up
                1
                ·
                3 months ago

                I can see the benefit of matching style when working with others. I only code for myself and never had to worry about conformity for project consistency.

                It is good to learn new things.

                I’m sure I have some coding habitats that would annoy others.

                • ScintillatingStruthio@programming.dev
                  link
                  fedilink
                  English
                  arrow-up
                  2
                  ·
                  3 months ago

                  Consistent styling helps make the actual meaningful changes easier to spot. Probably also useful for your own commit history when working solo in a repo, but most useful in a team, yeah!

  • DreamButt@lemmy.world
    link
    fedilink
    English
    arrow-up
    25
    ·
    3 months 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

  • ramble81@lemmy.zip
    link
    fedilink
    arrow-up
    11
    ·
    3 months ago

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

    • marcos@lemmy.world
      link
      fedilink
      arrow-up
      9
      ·
      3 months 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.

      • bjoern_tantau@swg-empire.de
        link
        fedilink
        arrow-up
        29
        ·
        3 months 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.

  • 9point6@lemmy.world
    link
    fedilink
    arrow-up
    6
    ·
    edit-2
    3 months 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

  • grrgyle@slrpnk.net
    link
    fedilink
    arrow-up
    1
    ·
    3 months 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.

  • ulterno@programming.dev
    link
    fedilink
    English
    arrow-up
    1
    arrow-down
    1
    ·
    3 months 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?

    • shape_warrior_t@programming.dev
      link
      fedilink
      English
      arrow-up
      5
      ·
      3 months 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.