DEV Community

Writing code for your future self

Sunny Singh on February 02, 2019

We've all been there. You write a piece of code, read through it, and think it's perfect because it makes sense to you at the time. Return to that ...

Read full post

Molly Struve (she/her) • Feb 2 '19

I would say, don't be afraid to leave a comment. Last week I came across some old code I wrote that I didn't think was needed. I yanked it out and was ready to issue a PR but luckily a spec caught an instance where we needed it. I added the code back and put a little comment to where the code was (not in an obvious way)referenced. If I had done that from the start I could have saved my current self a little bit of time and hassle.

Sunny Singh • Feb 2 '19

That's a perfect use case for a comment! I prefer code that is self descriptive but sometimes a comment is needed to explain why some code exists or was written a certain way.

TheRealZeljko • Feb 2 '19 • Edited

I would rather put that in the documentation of that function/method/class/whatever. I do not use comments in my code be it Python, Haskell, Java because I think that comments lie to you most of the time (I am a little biased towards this position after reading "Clean Code" from Robert Martin).

//EDIT
If I do not know what this code does after a month I need to rewrite it(assign better names for the values) or write better documentation

Tom Ashworth • Feb 3 '19

I am a junior developer. I agree in the principals of clean code and this is a great use case for a comment. With regards to moving it into documentation, I'd prefer to have a single source of information. It's easier to update a comment (yes, I'm aware they can become outdated) than documentation which has the same issue of becoming out dated.

Bartek Svaberg • Feb 7 '19

Why all the constants? I would rather treat a boolean and a function returning a boolean the same way when it comes to naming. In other words, instead of naming the function validateUserNameLength() I would name the it isUserNameLengthValid() or something similar. That would allow me to just go

if (!isUserNameLengthValid(userName) return

and skip all the constants.

And if you want to be really idiomatic about things just create a UserName type that has a function called hasValidLength(). That would allow you to write:

if (!userName.hasValidLength()) return

Mihail Malo • Feb 4 '19

// this is a user object or undefined, should probably use `some` instead of `find`
const isUserPostCreated = users[0] && posts.find(post => post.userId === users[0].id)

if (isUserPostCreated) {
  showUserPost()
}

I'd usually do the following:

const [user] = users
if (!user) return

const { id } = user

// Does the user have any posts?
if (!posts.some(({ userId }) => userId === id)) return

showUserPost()

I guess I could name that boolean instead of writing the comment (or doing nothing at all) but I'm just being honest, and this best represents what I usually write at this time in my life.
And indeed, naming is hard (I didn't find your name descriptive enough).

const [user] = users
if (!user) return

const { id } = user

const userHasPosts = posts.some(({ userId }) => userId === id)
if (!userHasPosts) return

showUserPost()

Not sure which is better. Comments go stale easier than names, but this pollutes the scope.
Maybe I should wrap the const+if into a block for lexical scoping.
That would be wild. Ha. Ha-ha. Ha.

Sunny Singh • Feb 5 '19 • Edited

I see no issues with reading your code, plus what I provided in the article are only meant to serve as suggestions. My own examples could definitely be improved as you showed.

I do prefer adding a variable over a comment though, because as you said: comments become stale.

But yeah, this stuff is hard sometimes 😅

Mihail Malo • Feb 5 '19

Variables become stale too :v
Don't tell me you have never seen something like

isValid.forEach(name(true))

They are easier to update though, since once someone finally realizes what that damn thing represents, they can apply a "rename identifier everywhere" refactoring tool.

Sunny Singh • Feb 5 '19

Haha, yeah good point. I agree that they are easier to update.

I like to use comments as a last resort to explain why a piece of code is written in a certain way or exists in the first place.

Vince Ramces Oliveros • Feb 3 '19

I would enlighten myself using function that returns a boolean keyword should followed by a camelCase in naming convention. I would like to thank Dart's linting that separates class, functions,private and public variables and collections(known for arrays in some languages).

Leave comments explaing "why" feature. I watched John Papa's video about "Writing Readable Code" and how it helped me learned cleaner and concise code.

Great post! I had the same experience as you did. Thanks!

Sunny Singh • Feb 3 '19 • Edited

It's great when languages have built-in constructs that help with writing more readable code.

Definitely agree on having comments that explain "why". I'll take a look at that video, thanks for sharing Vince!

Tyler Smith • Feb 2 '19

I LOVE the separate your conditions bit here. I never thought about doing that before but I'm gonna start doing this in all of my code.

Sunny Singh • Feb 3 '19

Super happy to hear this! Glad you were able to grab something actionable.

Aidas Bendoraitis • Feb 3 '19

Very good article! Thanks.

I just noticed that in the refactored code

return isLengthValid && isAlphanumeric && isUsernameTaken;

should rather be

return isLengthValid && isAlphanumeric && !isUsernameTaken;

Sunny Singh • Feb 3 '19

You're right, good catch! Maybe I should write unit tests for my article code blocks haha.

Mihail Malo • Feb 4 '19

And like in my other comment, I would suggest the early return pattern that has been refactored out of validateUsername be reintroduced.
Not just for the sake of short circuiting, which is important for performance if some of the later checks make additional network calls, but also because this pattern is always helpful as it doesn't force the developer to keep as many concepts in their head. (No more references to binding? Can forget concept.)

async function validateUsername(username) {
  if (!validateUsernameLength(username)) return false
  if (!validateAlphanumeric(username)) return false
  if (await checkUsernameExists(username)) return false
  return true
}

Comment deleted

Bartek Svaberg • Feb 7 '19

Deleted the one without love. =)

Marissa B • Feb 2 '19 • Edited

Great points about the naming! I find it easier to make improvements later when I can see not only what I did ages ago, but how I did it just as easily.

Sunny Singh • Feb 2 '19

Thanks Marissa! Exactly, being able to understand how your code is functioning definitely helps with future changes.

Nick Hiebert • Feb 2 '19 • Edited

It looks like our experiences are very similar. Making a function do one thing, making meaningful variable/function names. Simplicity is key for readability, I get really confused and I tend to have trouble understanding what's going on if the code is written poorly. Either from my past self or modifying a website that was built by someone else or a framework's code.

Sunny Singh • Feb 2 '19

Hi Nick! I'm happy to hear that. You've covered the key points of the article.

Mihail Malo • Feb 4 '19

There are only two hard things in Computer Science: cache invalidation and naming things.

I guess off-by-one errors are in the past since everyone switched to iterators/functional methods?

Sunny Singh • Feb 5 '19

Haha I was debating on including that but I don't think that's the original quote. Didn't seem relevant anyway.

Mihail Malo • Feb 5 '19

Well I have two comments on this post where I go on about how hard naming is, so the rest was definitely to the point.

Sebastian Śledź • Feb 17 '19

Hmm. Keep in mind that you can do more readable code without sacrificing performance. In last example you can still use functions directly in logical condition instead executing them and store result in temporary variable and code will be perfectly readable without querying DB when other checks fail. ;)

Sunny Singh • Feb 17 '19

Yep, I definitely agree in cases where the optimized version is just as readable.

Jesse Phillips • Feb 12 '19

I would challenge the idea that single responsibility means can't do more than one thing. Several have shown the challenge of refactoring into this approach.

The db access I would argue isn't related to username validation. I mean if it isn't valid how does someone have it.

pkassin • Feb 3 '19

Difference is that in first case validation will not db.query if username is too long. In second that db.query and other later tests are usuless

Sunny Singh • Feb 3 '19 • Edited

True. For the same functionality you would want something like this:

function validateUsername(username) {
  const isLengthValid = validateUsernameLength(username);

  if (!isLengthValid) return false;

  const isAlphanumeric = validateAlphanumeric(username);

  if (!isAlphanumeric) return false;

  const isUsernameTaken = checkUsernameExists(username);

  if (isUsernameTaken) return false;

  return true;
}

Or even this:

function validateUsername(username) {
  return validateUsernameLength(username)
    && validateAlphanumeric(username)
    && !checkUsernameExists(username);
}

Which would only run checkUsernameExists if the tests before it pass. But it's somewhat pseudo code anyway. The point of db.query was to show the single responsibility.

curquiza • Feb 3 '19

The fundamentals, well explained with examples 👌 Thank you !!