76
Self-documenting Code (lackofimagination.org)
top 50 comments
sorted by: hot top controversial new old
[-] Atlas_@lemmy.world 18 points 1 day ago* (last edited 1 day ago)

In addition to the excellent points made by steventhedev and koper:

user.password = await hashPassword(user.password);

Just this one line of code alone is wrong.

  1. It's unclear, but quite likely that the type has changed here. Even in a duck typed language this is hard to manage and often leads to bugs.
  2. Even without a type change, you shouldn't reuse an object member like this. Dramatically better to have password and hashed_password so that they never get mixed up. If you don't want the raw password available after this point, zero it out or delete it.
  3. All of these style considerations apply 4x as strongly when it's a piece of code that's important to the security of your service, which obviously hashing passwords is.
load more comments (6 replies)
[-] Simulation6@sopuli.xyz 131 points 1 day ago

Figuring out what the code is doing is not the hard part. Documenting the reason you want it to do that (domain knowledge) is the hard part.

[-] tatterdemalion@programming.dev 33 points 1 day ago

Agreed.

And sometimes code is not the right medium for communicating domain knowledge. For example, if you are writing code the does some geometric calculations, with lot of trigonometry, etc. Even with clear variable names, it can be hard to decipher without a generous comment or splitting it up into functions with verbose names. Sometimes you really just want a picture of what's happening, in SVG format, embedded into the function documentation HTML.

[-] hex@programming.dev 5 points 1 day ago* (last edited 1 day ago)

Yeah. I advocate for self explanatory code, but I definitely don't frown upon comments. Comments are super useful but soooo overused. I have coworkers that aren't that great that would definitely comment on the most basic if statements. That's why we have to push self explanatory code, because some beginners think they need to say:

//prints to the console
console.log("hello world");

I think by my logic, comments are kind of an advanced level concept, lol. Like you shouldn't really start using comments often until you're writing some pretty complex code, or using a giant codebase.

[-] Faresh@lemmy.ml 6 points 17 hours ago

Comments are super useful but soooo overused

I think overusing comments is a non-issue. I'd rather have over-commented code that doesn't need it, over undocumented code without comments that needs them. If this over-commenting causes some comments to be out of date, those instances should hopefully be obvious from the code itself or the other comments and easily fixed.

[-] hex@programming.dev 1 points 17 hours ago

I understand what you're saying and I mostly agree, but those few instances where a line of code is only slightly different and the comment is the same, can really be confusing.

[-] TehPers@beehaw.org 4 points 1 day ago

Sometimes when I don't leave comments like that, I get review comments asking what the line does. Code like ThisMethodInitsTheService() with comments like "what does this do?" in the review.

So now I comment a lot. Apparently reading code is hard for some people, even code that tells you exactly what it does in very simple terms.

[-] hex@programming.dev 4 points 1 day ago

Fair. I guess in this case, it's a manner of gauging who you're working with. I'd much rather answer a question once in a while than over-comment (since refactors often make comments worthless and they're so easy to miss..), but if it's a regular occurrence, yeah it would get on my nerves. Read the fuckin name of the function! Or better yet go check out what the function does!

[-] nous@programming.dev 2 points 22 hours ago* (last edited 21 hours ago)

Worse, refactors make comments wrong. And there is nothing more annoying then having the comment conflict with the code. Which is right? Is it a bug or did someone just forget to update the comments... The latter is far more common.

Comments that just repeat the code mean you now have two places to update and keep in sync - a pointless waste of time and confusion.

[-] hex@programming.dev 1 points 18 hours ago

Yes- exactly, they make comments wrong. But comments aren't always a waste of time, like in legacy code, or just in general code that isn't gonna change (mathematical equations too)

[-] nous@programming.dev 2 points 17 hours ago

Comments are not always a waste of time, but comments that repeat or tell you what the code is doing (rather than why) are a waste. For legacy code you generally don't have comments anyway and the code is hard to read/understand.

But if you can understand the code enough to write a comment you can likely refactor the code to just make it more readable to start with.

For code that does not change generally does not need to be read much so does not need comments to describe what it is doing. And again, if you understand it enough to write a comment to explain what it is doing you can refactor it to be readable to begin with. Even for mathematical equations I would either expect the reader to be able to read them or link to documentation that describes what it is in much more detail to name the function enough that the reader can look it up to understand the principals behind it.

[-] hex@programming.dev 1 points 17 hours ago

You make some great points. Using smaller functions and breaking up your code in readable bits makes a huge difference and you will likely never need comments if you do it right 👍🏻

[-] nous@programming.dev 3 points 15 hours ago

Creating functions is IMO not the first thing you should do. Giving variables better names or naming temporaries/intermediate steps is often all you really need to do to make things clearer. Creating smaller functions tends to be my last resort and I would avoid it when I can as splitting the code up can make things harder to understand as you have to jump around more often.

[-] hex@programming.dev 1 points 14 hours ago

I hear ya. As always, it's a balance between having functions that are too long, and many too small functions. Matter of team preferences too.

[-] Flamekebab@piefed.social 10 points 1 day ago

TempleOS: Hold my communion wine

[-] steventhedev@lemmy.world 19 points 1 day ago

One upvote is not enough.

I once wrote a commit message the length of a full blog post comparing 10 different alternatives for micro optimization, with benchmarks and more. The diff itself was ten lines. Shaved around 4% off the hot path (based on a sampling profiler that ran over the weekend).

load more comments (1 replies)
[-] steventhedev@lemmy.world 61 points 1 day ago

Ew no.

Abusing language features like this (boolean expression short circuit) just makes it harder for other people to come and maintain your code.

The function does have opportunity for improvement by checking one thing at a time. This flattens the ifs and changes them into proper sentry clauses. It also opens the door to encapsulating their logic and refactoring this function into a proper validator that can return all the reasons a user is invalid.

Good code is not "elegant" code. It's code that is simple and unsurprising and can be easily understood by a hungover fresh graduate new hire.

[-] sip@programming.dev 1 points 13 hours ago* (last edited 13 hours ago)

assert(isPasswordGood(...)) is already in ~~the language.~~ node

[-] traches@sh.itjust.works 44 points 1 day ago

Agreed. OP was doing well until they replaced the if statements with ‚function call || throw error’. That’s still an if statement, but obfuscated.

Don't mind the || but I do agree if you're validating an input you'd best find all issues at once instead of "first rule wins".

[-] rooster_butt@lemm.ee 2 points 16 hours ago

Short circuiting conditions is important. Mainly for things such as:

if(Object != Null && Object.HasThing) ...

Without short circuit evaluation you end up with a null pointer exception.

[-] hex@programming.dev 5 points 1 day ago

I mean, boolean short circuit is a super idiomatic pattern in Javascript

[-] arendjr@programming.dev 5 points 17 hours ago

I think that’s very team/project dependent. I’ve seen it done before indeed, but I’ve never been on a team where it was considered idiomatic.

[-] hex@programming.dev 1 points 17 hours ago

That makes sense.

[-] clutchtwopointzero@lemmy.world 2 points 20 hours ago

Because on JS the goal is to shave bytes to save money on data transfer rates

[-] hex@programming.dev 1 points 18 hours ago

It's not that deep. It looks nice, and is easy to understand.

[-] YaBoyMax@programming.dev 12 points 1 day ago

This is the most important thing I've learned since the start of my career. All those "clever" tricks literally just serve to make the author feel clever at the expense of clarity and long-term manintainability.

[-] verstra@programming.dev 19 points 1 day ago

I agree, this is an anti-pattern for me.

Having explicit throw keywords is much more readable compared to hiding flow-control into helper functions.

[-] lmaydev@lemmy.world 8 points 1 day ago

100% un-nesting that if would have been fine.

load more comments (1 replies)
[-] koper@feddit.nl 37 points 1 day ago* (last edited 1 day ago)

Why the password.trim()? Silently removing parts of the password can lead to dangerous bugs and tells me the developer didn't peoperly consider how to sanitize input.

I remember once my password for a particular organization had a space at the end. I could log in to all LDAP-connected applications, except for one that would insist my password was wrong. A trim() or similar was likely the culprit.

[-] spechter@lemmy.ml 29 points 1 day ago

Another favorite of mine is truncating the password to a certain length w/o informing the user.

[-] NotationalSymmetry@ani.social 15 points 1 day ago

Saving the password truncates but validation doesn't. So it just fails every time you try to log in with no explanation. The number of times I have seen this in a production website is too damn high.

load more comments (2 replies)
[-] Flipper@feddit.org 9 points 1 day ago

The password needs to be 8 letters long and may only contain the alphabet. Also we don't tell you this requirement or tell you that setting the password went wrong. We just lock you out.

[-] HamsterRage@lemmy.ca 12 points 1 day ago

The reason for leaving in the password.trim() would be one of the few things that I would ever document with a comment.

[-] Aijan@programming.dev 15 points 1 day ago* (last edited 1 day ago)

Thanks for the tip. password.trim() can indeed be problematic. I just removed that line.

[-] Rogue@feddit.uk 6 points 1 day ago

A quick glance and this seemed nothing to do with self documenting code and everything to do with the flaws when code isn't strictly typed.

load more comments
view more: next ›
this post was submitted on 21 Oct 2024
76 points (85.2% liked)

Programming

17241 readers
256 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities !webdev@programming.dev



founded 1 year ago
MODERATORS