8

I was wondering why my tutor needs a whole ass week to accept my MR.

Today he rejected one, so I got a chance to look at whatever he's doing.

He's checking line by line every single test I make and creates a variable for each dumb thing.

𝘦𝘹𝘱𝘦𝘤𝘵(𝘴𝘰𝘮𝘦𝘵𝘩𝘪𝘯𝘨.𝘪𝘥).𝘵𝘰𝘚𝘵𝘳𝘪𝘤𝘵𝘌𝘲𝘶𝘢𝘭(𝘴𝘰𝘮𝘦𝘖𝘵𝘩𝘦𝘳𝘛𝘩𝘪𝘯𝘨.𝘪𝘥)? No, this is bullshit.

𝘤𝘰𝘯𝘴𝘵 𝘴𝘰𝘮𝘦𝘵𝘩𝘪𝘯𝘨𝘐𝘋 = 𝘴𝘰𝘮𝘦𝘵𝘩𝘪𝘯𝘨.𝘪𝘥
𝘤𝘰𝘯𝘴𝘵 𝘴𝘰𝘮𝘦𝘖𝘵𝘩𝘦𝘳𝘛𝘩𝘪𝘯𝘨.𝘪𝘥 = 𝘴𝘰𝘮𝘦𝘖𝘵𝘩𝘦𝘳𝘛𝘩𝘪𝘯𝘨.𝘪𝘥
𝘦𝘹𝘱𝘦𝘤𝘵(𝘴𝘰𝘮𝘦𝘵𝘩𝘪𝘯𝘨𝘐𝘋).𝘵𝘰𝘚𝘵𝘳𝘪𝘤𝘵𝘌𝘲𝘶𝘢𝘭(𝘴𝘰𝘮𝘦𝘖𝘵𝘩𝘦𝘳𝘛𝘩𝘪𝘯𝘨𝘐𝘋)

I don't even know why you would take a week to accept a merge request when all you're doing is creating variables for things you use only once. I'm not even mad, I'm not ranting, I just need to know why would you do such a thing

Comments
  • 1
    I would definitely be frustrated, if not mad if someone did that to the code I wrote. I can understand if the point is to make things more readable. But what he’s doing here is an absolute waste of time and it makes it longer to read
  • 7
    Setting something to a variable for no real purpose, is pointless and just increases the memory footprint.

    Don't get me wrong, the odd variable here and there won't do much, but everything as it's own... that's just duplicate data lying around for no good reason.

    But then again, JS devs rarely know anything about memory optimisation so makes sense. 😏
  • 1
    What kinda thing are you writing?

    Also, lol unit tests.
  • 1
    @C0D4 I mean, who cares of the memory footprint tbh, I'm in JS and no random allocation will ever be a problem, especially in a unit test. But we have this nice little things called objects, they contain all the data about any given entity. I would understand if you decided to go the other way around (as in: from his code to mine, so everything's packed), or not do anything at all (since his code is not even wrong). But going out of your way specifically to spread variables around is a new.

    @atheist integration tests, there's a huge amount of endpoints nobody's keeping an eye on
  • 6
    @IHateForALiving "who cares of the memory footprint"

    Maybe one day you'll work with something that cares.

    But when I need 32GB of Ram to open a todo list, meh, you're right who cares 🤷‍♂️
  • 3
    @C0D4 Constants that are only used once are inlined, as have they been for the past 20 years.
  • 0
    @lbfalvy or in other words:

    Don’t try to outsmart the compiler. Or at least give it some credit.
  • 2
    @C0D4 let's contextualize this: we're talking about a unit test for a NodeJS app. It's gonna run once per day inside a Docker container which is then destroyed. It could take a whole night to run on its dedicated machine and nobody would care.

    Memory optimization is important and all, but if you reach the point when you're cutting a single memory allocation like it matters, things went very wrong a long ago. 99% of the time readability is what matters, if I cared about blazing light performances I wouldn't be working on a NodeJS app. So long as you avoid very blatant errors (eager fetching your queries, most of the times) you should think about readability and be good for 99% of use cases. When things go wrong, you optimize what's wrong.
  • 1
    The iOS devrant client did something weird to your rant preview 😯
  • 1
    @Lensflare It probably cuts the preview by bytes and he's using unicode itallics.
  • 0
    I wonder what would happen if I got the client to print a transmission control byte at the end of the preview.
  • 0
    Or a null. It's probably easier to produce a null and I know that sad excuse of an OS doesn't like them.
  • 0
    @lbfalvy smells like a possible way to hack it.
  • 1
    @Lensflare Not really, the entire vulnerability is a single byte at the end of a cleartext string which can be set to values otherwise filtered by the input sanitizer. At worst it can produce an immediate crash.
  • 0
    Sounds like they're way overdoing it but I sometimes do something similar in tests to create readable code. Just that it's clear what exactly you're verifying (simplified example)

    String formattedFullName = foo. getFirstName() + " " + foo.getLastName();

    expect(formattedFullName).equals(foo.getFullName())
  • 1
    Actually you know...
    If u use something like

    "If(X less than 2)
    Do this"

    Renaming to
    "If servers amount is less than server limit
    Do this"
    Makes a great sense in order to remove "magical numbers", which we have no idea what they are doing there
  • 0
    @darkwind yup. It can be used as documentation. But with code instead of comments.
Add Comment