DEV Community

loading...

4 practices for better code

Klaudia
Software Developer.
・4 min read

This post was originally posted on my blog, find it here.

"Clean code always looks like it was written by someone who cares."

I remember my first job as web developer. It was shortly after I gratuated from bootcamp and I found myself in a role where every developer was on his own and the only reviews I've gotten were from designers who checked whether it matched the designs and behaved as expected.

That was until I - inevitably - got stuck on one of the projects. One of the senior developers tried to debug my code and then he realized just how junior I was. In an attempt to make me a better developer and less of a pain in the ass for him, he came to me after, gave me a book and told me to read it.

It was Clean Code by Robert Cecil Martin.

I can honestly say that this book is responsible for a huge shift in my thinking. I started to look at the code differently. Lots was said but few principles I will remember as long as I'll code. Here are the 4 that stuck with me the most and the ones I consider to be of the utmost importance to any developer or engineer.

Careful with the names

Be careful what you name your variables and functions, classes, files, basically anything and everything. Names should be descriptive of what the variables is or what the function does. This helps you read the code easier and it's more obvious what the code does on the first read without much investigation.

🤢BAD

const a = document.querySelector('.a');
const b = document.querySelector('.b');
const w = window.innerWidth / 2;
const h = window.innerHeight / 6 ;

b.addEventListener('mousemove', (e) => {
  const x = e.clientX / w;
  const y = e.clientY / h;
  a.style.transform = `translate3d(-${mouseX}%, -${mouseY}%, 0)`;
  b.style.transform = `translate3d(${mouseX}%, ${mouseY}%, 0)`;
});
Enter fullscreen mode Exit fullscreen mode

🌟GOOD

const bg = document.querySelector('.background');
const bgTop = document.querySelector('.background-top');

const windowWidth = window.innerWidth / 2;
const windowHeight = window.innerHeight / 6 ;

bgTop.addEventListener('mousemove', (e) => {
  const mouseX = e.clientX / windowWidth;
  const mouseY = e.clientY / windowHeight;

  bg.style.transform = `translate3d(-${mouseX}%, -${mouseY}%, 0)`;
      bgTop.style.transform = `translate3d(${mouseX}%, ${mouseY}%, 0)`;
});
Enter fullscreen mode Exit fullscreen mode

It shouldn't be too long, should be descriptive and the name format should be consistent across the code base.

🤢BAD

function setTheValueOfSomethingAndReturnAValueToDoSomething() {
    ...
}
function addSomethingElseToThatThing() {
    ...
}
Enter fullscreen mode Exit fullscreen mode

🌟GOOD

const getTime = () => { ... };
const getDate = () => { ... };
const setNewDate = () => { ... };
Enter fullscreen mode Exit fullscreen mode

If there is a naming convention in the language, use it. Javascript's got camelCase, python is using_underscores, other language got theirs. Naming in line with conventions helps you and helps your fellow developers looking after the code base as well.

WET is bad, WET is bad aka DRY is gold

DRY - Don't repeat yourself is one of the best practices you can follow. If you find yourself WET: Writing Everything Twice and Wasting everyone's time, abstract it and make your live easier and code considerably shorter.

🤢BAD

const goodPractices = {
  dry: true,
  wet: false,
  kiss: true,
  descriptive: true
}

const getDRY = () => goodPractices.dry;
const getWET = () => goodPractices.wet;
const getKISS = () => goodPractices.kiss;
const getDescriptive = () => goodPractices.descriptive;

Enter fullscreen mode Exit fullscreen mode

🌟GOOD

const goodPractices = {
  dry: true,
  wet: false,
  kiss: true,
  descriptive: true
}

const getParam = (param) => goodPractices[param];
Enter fullscreen mode Exit fullscreen mode

Keep it simple, stupid

This tweet sums KISS principle better that I ever could:
Liquid error: internal
Basically, KISS is about making your code readable and simple. Adding only things you need, not things that look smart. Let's compare two solutions for one of the katas I found on Code Wars

🤢BAD

H=(Q,S)=>Q.map(V=>null==V||(V.map?H(V,S):'object'==typeof V?H(Object.values(V),S):/nu|st/.test(typeof V)&(V=+V)==V&&++S[S[0]+=V,1]))
averageEverything=(...Q)=>H(Q,Q=[0,0])&&Q[0]/Q[1]
Enter fullscreen mode Exit fullscreen mode

🌟GOOD

const plus = (v,w) => v+w ;
const length = a =>
  typeof a==="number" ? Number.isNaN(a) ? 0 : 1 :
  typeof a==="string" ? Number.isNaN(Number(a)) ? 0 : 1 : // or just `isNaN(a)`
  typeof a==="object" ? Object.values( a || [] ).map(length).reduce(plus,0) :
                        0 ;
const sum = a =>
  typeof a==="number" ? a || 0 :
  typeof a==="string" ? Number(a) || 0 :
  typeof a==="object" ? Object.values( a || [] ).map(sum).reduce(plus,0) :
                        0 ;
const averageEverything = (...a) => sum(a) / length(a) ;
Enter fullscreen mode Exit fullscreen mode

Now imagine there is a bug (and trust me, there will be bugs) and you need to fix it. Where do you even start with the top one? Sure, it looks genius and it's 3 lines, but at what cost? Meanwhile, the bottom example might be 10 lines longer but when you read it you have an idea of what's going on. I'm not saying ALWAYS make your code longer BUT you should try and make it as readable as possible.

Longer is not always better

... when it comes to functions. I'm not saying you should keep your functions small no matter what if makes no sense or if it's just one line that return the sum of two numbers, but long functions usually mean that they do more than they should.

"Functions should do something, or answer something, but not both."

Basically, if you see your function does something that can be potentially extracted and put into a separate function, you should do it. This also makes testing much easier.

🤮BAD

const submitHandler = e => {
  e.preventDefault();
  postApiData('/login', {username, password})
   .then(user => {
    if (user && user.token) {
     Cookies.set('token', user.token)
   }
   return user;
   });
  if (!this.state.authError) {
   this.props.history.push('/theories')
  }
}
Enter fullscreen mode Exit fullscreen mode

🌟GOOD

const redirect = (path) => this.props.history.push(path);    
const login = (username, password) => {
  return postApiData('/login', {username, password})
    .then(user => {
      if (user && user.token) Cookies.set('token', user.token);
    });
}

const submitHandler = e => {
  e.preventDefault();
  login(username, password);
  if (!this.state.authError) redirect('/somewhere');
}
Enter fullscreen mode Exit fullscreen mode

Parting Words of Wisdom

Of course, all this is nice but don't take this as a law. Use your head and use this principles when applicable. I do hope this helps you on your journey to a better code!

Liquid error: internal

Discussion (20)

Collapse
codevault profile image
Sergiu Mureşan • Edited
const goodPractices = {
  dry: true,
  wet: false,
  kiss: true,
  descriptive: true
}

const getParam = (param) => goodPractices[param];

This works in theory, if your object is const and instantiated right near the function getParam otherwise it might be difficult to know which fields you have in the object and which you don't. Especially if the object is not const

Using comments might be a temporary fix but if someone in your team modifies the properties of the object they will have to know that the getParam function has a comment listing all properties.

Ok, bear with one more of my critiques with

const redirect = () => this.props.history.push('/theories');    
const login = (username, password) => {
  return postApiData('/login', {username, password})
    .then(user => {
      if (user && user.token) Cookies.set('token', user.token);
    });
}

const submitHandler = e => {
  e.preventDefault();
  login(username, password);
  if (!this.state.authError) redirect();
}

This is actually a good example, the concept can't be easily put into practice without hurting readability.

Functions should do something, or answer something, but not both

Yes, but they should be highly reusable. If the function itself is only used in this context it will be much harder to understand the code itself than if it was inside the submitHandler function.

Look at it this way, to understand submitHandler you will now have to understand both login and redirect. Okay, login here makes sense but redirect is oddly specific, maybe if it had parameters and was a utility function.

The rest of the examples are really good and, aside from some edge cases, they will work most of the time. Especially the first one, I learned it the hard way from a senior colleague :D

Thank you for the post. What do you think?

Collapse
effingkay profile image
Klaudia Author

Great great points! Thank you so much for raising them, you actually picked up what I was worried about when I wrote the examples.

I have nothing else to say about your first point other than you're absolutely right. There can be some issues in the future with this code but then again - this was an artificial example just to demonstrate the point so I wasn't that worried about it

Second - redirect was a utility function actually, I dunno when I changed it but oh well, I'll go and correct it, great catch - thanks for noticing. And yes, submitHandler is harder to understand now but I think login and redirect are nice, descriptive names so you know what's happening when you're reading the code, plus both login and redirect are used in multiple places so they need to be utilities.

Thank you again for commenting!

Collapse
tsuzukayama profile image
Tiago Suzukayama

only part I don't really agree with is that long names for functions are bad. Here at work we don't use abbreviations, and it really makes finding out what a function does as easy as reading its name.

Collapse
effingkay profile image
Klaudia Author

That is a good point actually, I haven't thought of that but counterpoint - if you need a long name to describe what a function does, doesn't it mean that it does too much? I'm sure there's exceptions where long name is necessary but usually, shouldn't you need a few words to describe a function or variable? Genuine question

Although yes - abbreviations do suck so I agree, longer name is better imo

Collapse
mikeschinkel profile image
Mike Schinkel

I agree Klaudia.

In our shop we have a convention that is you need to use more than one underscore (we do PHP with snake_case) then you are probably trying to do too much and should probably refactor it. But then our shop is highly convention driven, so we are probably more pedantic about it than most.

Collapse
lluismf profile image
Lluís Josep Martínez

I agree, abbreviations usually suck. Function/method names should be as long as necessary, not shorter.

Collapse
imikemiller profile image
Mike Miller

I agree. I don't see the negative. Your IDE will autocomplete so there is no overhead

Collapse
rvprasad profile image
Venkatesh-Prasad Ranganath

To help learn good software development practice, I give students The Art of Readable Code and Pragmatic Programmer.

Collapse
tux0r profile image
tux0r

getWET, getKISS. Ha!

One important thing missing here: Write good comments!

Collapse
hellsinglord22 profile image
H (إتش ) • Edited

i disagree, comments usually lie .
See code get updated all the time but there is no restriction on comments so it can mislead you .
i think you better put all the effort in writing readable code and only use comments with

  1. license
  2. explaining an algorithm
  3. Generate documentation using JSdocs or whatever tool your are using .
Collapse
tux0r profile image
tux0r

explaining an algorithm

Precisely.

Thread Thread
spikespaz profile image
Jacob Birkett

I think he's saying don't add comments on every single line.

/// Gets the last item index used in the current state.
size_t getLastIndex() {
    [...]
    // Returns the last index.
    return lastIndex;
}

The doc comment at the top of the function already says what the last line if the function body is, yet I see things like this all the time.

Collapse
effingkay profile image
Klaudia Author

Good point! I wholeheartedly agree 👍
But comments deserve their own article, there's so much to tell about commenting :)

Collapse
joshichinmay profile image
Chinmay Joshi

How about writing self-explanatory code instead?

Collapse
mikeschinkel profile image
Mike Schinkel • Edited

Hi Klaudia,

Great post. But I do want to address one point:

"Basically, if you see your function does something that can be potentially extracted and put into a separate function, you should do it."

While in general I agree, I do want to make a counter-point, and one I think far too few developers pay attention to. Serendipitously, it aligns with another point you made:

"Be careful what you name your variables and functions, classes, files, basically anything and everything."

The counter-point is this: If you subdivide a function into two or more functions you now need to come up with good names for the additional functions, and that can often be difficult.

After 25+ years programming I've come to believe that the best names:

  1. Follow objective naming conventions that anyone who follows correctly would use the same names,

  2. That are repeatable by which I mean that that I or someone else would use the same name in the future if we needed to do the same thing again.

If you follow those two rules in your team's codebase you will end up with much more cohesive code. But that also argues for restraint in creating new symbol names, unless you really need to, and ideally when you have an objective set of rules that apply for the naming.

#jmtcw

-Mike

Collapse
vitalcog profile image
Chad Windham

Hey great article! Thanks for taking the time to write that up!

Collapse
effingkay profile image
Klaudia Author

I know, that code weirded me out as well but it works I guess...? :D

Collapse
doniz profile image
Donatas Navidonskis

This can be improved. On the end part use dependency injection for login which integrates into submitHandler. And the error block you can move within promise catch.