DEV Community

Adrian Carter
Adrian Carter

Posted on

 

Open Discussion: What Tips Do You Have For Writing Clean Code?

Recently, I read an Article where the author, Jerry Low, suggested that developers write their CSS properties in alphabetical order. I think this is a great idea, and it got me thinking what some other tips for writing clean and readable code? Feel free to share your suggestions.

Top comments (42)

Collapse
 
paratron profile image
Christian Engel

I read that article out of curiosity and I have to say, I am more than just a bit irritated by it.

The article is a perfect example of someone selling a personal preference as if its a fact which everyone should obey.

The autor even admits that after doing some research he found out that just about 10% of other developers follow his example and the majority does logical grouping.

I have to say, I think alphabetical oordering is just as bad as random ordering. The two examples looked exactly the same for me. I am very much in favor of the logical grouping (with blank lines in between!) and think its much easier to scan over.

But still: is is preference and not "clean code".

If you think about improving your general coding abilities, I can warmly recommend two books:

A philosophy of software design
by John Ousterhout

Refactoring (second edition)
by Martin Fowler

I learned a lot out of these two books.

Collapse
 
adrvnc profile image
Adrian Carter • Edited

Thanks for sharing your perspective, it's cool to read an opposing viewpoint on this article. Everyone has different personal preferences as it pertains to the way they code. After reading your comment, I can understand why the article can rub some people the wrong way.

Collapse
 
jeremyf profile image
Jeremy Friesen

I like to think of this in radiating bits of advice. First rule: no methods may be longer than 10 lines. From that rule many things follow.

I then look to Sandi Metz's guidance:

  1. No class must be greater than 100 lines
  2. No method must be greater than 5 lines
  3. No more than 4 parameters in a method
  4. Controllers can instantiate only one instance variable

Or look to literate programming. Or command/query separation. Or the Law of Demeter.

But focus on small methods and these things begin to emerge.

Collapse
 
jeoxs profile image
José Aponte

wow, "no method must be greater than 5/10 lines".. So, basically most complex methods will be composed of methods only? I can imagine a class having 20 more methods because of this rule 🤔. It is true that one method shouldn't have too many lines, but going the very opposite of 5 lines is just too extreme.

I can agree on the "no more than 4 parameters in a method" rule. This makes more sense than the extreme methods lines limit rule.

I can rescue and agree with you totally on what you say at the end: Focus on making smaller methods and the rest will follow.

Collapse
 
darkwiiplayer profile image
𒎏Wii 🏳️‍⚧️
  1. Don't be lazy about typing. Abbreviations harm readability.
  2. Don't encode data in your control flow. switch/case is evil.
  3. Don't implement what you need right now. Think in abstracts.
Collapse
 
mephi profile image
mephi

I don't get the problem with switch case. Can you provide an example?

Collapse
 
darkwiiplayer profile image
𒎏Wii 🏳️‍⚧️

switch/case is often abused to express data that should be a map as control flow instead. Then people wonder why their code is so hard to extend.

Thread Thread
 
mephi profile image
mephi

Ok... but if I don't use Python, I can't put everything in a map.

Thread Thread
 
darkwiiplayer profile image
𒎏Wii 🏳️‍⚧️

Yes, you can? What programming language doesn't support maps these days?

Collapse
 
tqbit profile image
tq-bit • Edited

I recently wrote all of these rules down for myself. The document is more than 600 lines long, so here are my 3 most important tldrs. All of them are inspired by Uncle Bob's best seller.

1. Write code so it resembles its own specification

I find this the most relevant point, but also the hardest. I got the idea by combining the rule 'Use intention-revealing names' from Clean Code and 'Start Statements with Verbs' and 'Be brief' from Microsoft's writing guidelines.

A bit easier to imagine is: Write code like you write a newspaper article. A common rule here is The Pyramid Principle of writing. So all those essays in highschool were actually good for something

2. Prefer readability over performance

You might have noticed there's a pattern already. Code must be read by people, not machines. In most of the cases, the fancy JS 1-liners come with a trade-off in readability. MS writing guidelines support this - 'Write like you speak' tells you to write like a person, not a machine.

3. Do proper error handling and always do it the same way

The idea for this comes from the chapter 'Use special error objects'.

My solution is to use try-catch inside a custom function that always returns an error-tuple.

In Typescript, the type looks like this:

type TryCatchTuple<T> = [T | null, Error | null];
// Resolves in a 2-element array with [result, error]
// -> result is null if there's an error
// -> error is null if there's a result
Enter fullscreen mode Exit fullscreen mode

It's used in this function (async example):

const tryCatchAsync = async <T>(fn: () => Promise<T>, ): Promise<TryCatchTuple<T>> => {
    try {
        const result = await fn()
        return [result, null];
    } catch (error) {
        return [null, error];
    }
}
Enter fullscreen mode Exit fullscreen mode

For sync, just leave out the async/await and the Promise generics.

Collapse
 
squirrelcoding profile image
Squirrelcoding

I don't like point #2, there's source code and compiled code. Those fancy "JS one-liners" aren't even meant to be read, they're optimized and compiled source code. Best practices and idiomatic code can lead to good readability and performance.

Collapse
 
tqbit profile image
tq-bit • Edited

Maybe I've phrased myself poorly here. What I meant to say is: I'd prefer function code like this:

const numbers = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];

const doubleNumbers = numbers.map(number => number * 2);
const numbersAboveFive = doubleNumbers.filter(number => number > 5);
const numberSum = numbersAboveFive.reduce((acc, number) => acc + number, 0);
Enter fullscreen mode Exit fullscreen mode

over this one:

const numbers = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];

const numbersDoubledAboveFiveSum = numbers
    .map((number) => number * 2)
    .filter((number) => number > 5)
    .reduce((acc, number) => acc + number, 0);
Enter fullscreen mode Exit fullscreen mode

in more complex settings.

I've seen a 50-liner in a productive codebase during last hacktoberfest and I found it rather hard to decipher due to these constructions.

If you chain the above example to a single function called duplicateNumbersAndSumAllAboveFive, fine by me. If you do three other things along the way, you should think about refactoring. Or at least make debugging less messy.

Thread Thread
 
mephi profile image
mephi

To me the second example is much easier to read, as I don't have to make the connection between the four variables on my own. I can read it like "ok, I want numbers doubled above five and summed up. I take the numbers, map to their double, filter all above 5 and apply a standard sum reduce.

Thread Thread
 
tqbit profile image
tq-bit • Edited

Then let me give you another example.

The following code snippet is meant to provide state for a react component (this - context = applicationwide state). I have slightly changed the original variable names, otherwise it's the same logic.

I then gave myself 10 minutes to try and refactor the function. When I put both into my text editor and try to debug, the second function is surely easier on the eye than the first. I might be wrong though, that's just my opinion.

Original

function colorsByCategory() {
    const colorsByCategory = this.item?.colorsByCategory;

    if (!colorsByCategory) {
        return {};
    }

    const categories = {};
    const availableColors = this.initialState.permittedColors;
    const allowedColors = this.configuration.allowedColors;

    Object.keys(colorsByCategory).forEach((key) => {
        const category = { ...colorsByCategory[key] };
        const colors = category.colors
            .filter((color) => availableColors?.includes(color.colorKey))
            .map((color) => {
                color.disabled = !allowedColors?.includes(color.colorKey);
                return color;
            });

        if (colors.length > 0) {
            category.colors = colors;
            categories[key] = category;
        }
    });

    return categories;
}
Enter fullscreen mode Exit fullscreen mode

My 'debugable' approach

function colorsByCategory() {
    const itemColorsByCategory = this.item.colorsByCategory;
    const availableItemColors = this.initialState.permittedColors;
    const allowedItemColors = this.configuration.allowedColors;

    return !!itemColorsByCategory ? getItemColorCategories() : {};

    function getItemColorCategories() {
        let categoryMap = {};

        Object.keys(itemColorsByCategory).forEach((key) => {
            const itemCategoryEntry = cloneEntry(itemColorsByCategory, key);
            const permittedItemColors = filterPermittedColors(itemCategoryEntry.colors);
            if (permittedItemColors.length > 0) {
                categoryMap = assignColorToCategory(categoryMap, itemCategoryEntry, key);
            }
        });

        return categoryMap;

        function assignColorToCategory(categoryMap, categoryEntry, key) {
            const localCategory = cloneEntry(categoryEntry);
            categoryMap[key] = localCategory;
            return categoryMap;
        }

        function filterPermittedColors(colors) {
            return colors
                .filter((color) => availableItemColors?.includes(color.colorKey))
                .map((color) => {
                    color.disabled = !allowedItemColors?.includes(color.colorKey);
                    return color;
                });
        }

        function cloneEntry(category, key) {
            return key ? { ...category[key] } : { ...category };
        }
    }
}
Enter fullscreen mode Exit fullscreen mode
Collapse
 
jonrandy profile image
Jon Randy 🎖️
  1. Remember "Clean Code" is subjective
  2. Don't prioritise "cleanliness" over efficiency
Collapse
 
darkwiiplayer profile image
𒎏Wii 🏳️‍⚧️

Efficiency in what sense though? Clean code is usually easier to maintain, making work more efficient.

Collapse
 
jonrandy profile image
Jon Randy 🎖️ • Edited

I've been in code reviews where code was rejected in favour of "cleaner" code, despite being many orders of magnitude faster. Later, clients complain about performance. Madness

Thread Thread
 
darkwiiplayer profile image
𒎏Wii 🏳️‍⚧️

But those cases are often just a false dichotomy. Usually, it works something like this:

  • The naive bad solution looks clean
  • The naive good solution looks messy
  • The good solution can look clean with the right abstractions

The right solution is obviously to spend the extra time to figure out how to make the good solution actually look clean, but if you ignore that option entirely, you can claim to decide between performance and readability.

When people find problems that really can't be expressed cleanly without a massive performance loss, they're either writing a C compiler, or they haven't spent enough time looking for a better solution.

Thread Thread
 
jeoxs profile image
José Aponte

I follow the 3 "make" rule when programming:

  1. Make it run!
  2. Make it good!
  3. Make it fast!

The clean code will appear between the steps 2 & 3.

Collapse
 
jmau111 profile image
jmau111⭐⭐ • Edited

To me, it's usually a triangle: Efficiency, readability, abstraction.
However, it's rarely the same distribution, as it can vary from one project to another, depending on the context. I can't give a general rule but this how I see the project in terms of clean code.

You can't neglect performances but you have to avoid micro-optimization, especially when the code becomes too "ninja", a.k.a. less readable and maintainable. If you're the only one who can modify the script, it can become a problem quickly.

Too much abstraction is not really good in my experience. It can jam code reviews significantly. Besides, you can get a lot of unwanted side effects, so more stress for maintenance, etc.

Collapse
 
gustavofsantos profile image
Gustavo Santos

About the article, I feel that is more personal taste than some pragmatic thing. There is a big difference between opinion (which everyone has) and well-known metrics such as SLOC, Cyclomatic Complexity and so on.

As tips, I suggest reading those books:

As a general rules of thumb that I like to follow:

  • Follow Tell, Don't Ask when possible, but not become too dogmatic about that
  • Law of Demeter is not a law, but it helps drive the code to a good and maintainable future
  • Start writing acceptance (e2e) tests, then write unit/integration tests, then write the code. This helps a lot organizing ideas and communication between the parts, without writing all the base classes upfront.
  • ReadMe driven development help organize what to do first and this impact directly to the code organization
  • Know your tools, this help you to use your tools to write your code.
  • Use dependency injection, even if it needs to be done in a manual way.
  • Write code for humans, not for machine.
  • (questionable taste) Switch/case isn't evil. Use it in factories to help to use polymorphism
  • (questionable taste) Do not put repeated prefixes or suffixes, like "UseCase", "I", "Service", "Entity" and so on (unless if needed to work, like the prefix use in React Hooks). Each object should tell a story, each object should deserve its place in the source code by the story that it tells, not by its name.
Collapse
 
paratron profile image
Christian Engel

Yes, definitely. The rule of thumb needs to be: quick to understand. If too complicated, break it up.

Thread Thread
 
darkwiiplayer profile image
𒎏Wii 🏳️‍⚧️

For trivial code, yes, but some problems have a certain irreducible complexity that cannot be reasonably split. As long as a method does exactly one thing, it's okay. If it does more than that, even if it's a two-liner, you're doing something wrong.

Collapse
 
taijidude profile image
taijidude • Edited

Imagine while coding the colleague who has to maintain the code after you is a violent psychopath who knows where you live. 😅

Collapse
 
mephi profile image
mephi

To me, a clean code rule has to be something that is worth the effort in the end.
I'm not that fast at alphabetical sorting, so I would take a lot of extra time and it would exceed the benefits it could provide.
My favourite type of sorting is the one my IDE does on autoformat.

Collapse
 
bwca profile image
Volodymyr Yepishev

The more articles I read on "clean code", the more it reminds me of a religion or a cult :)

 
jeremyf profile image
Jeremy Friesen

A good writer knows the rules and when and why to break them.

Collapse
 
gass profile image
Gass • Edited

hmm.. I don't like that approach. I've never tried it but I feel like that would slow me down. I preffer modular and small css files. Another thing I like doing is using the id attribute to distinguish the different sections. So I can write styles like so #info_card .title which makes it easier to find the section I need to work in.

Collapse
 
pinotattari profile image
Riccardo Bernardini

Writing CSS properties in alphabetical order? I fail to see how this can make the code clearer; maybe it helps when you are searching for a property, but Ctrl-F (or Ctrl-s if you are an emacsian) works fine too.

To me "clean code" means code that makes it easier to understand what its goal is and how it achieves it. In this respect, I find that using meaningful names for variables, functions, etc. helps a lot. Using some comment to explain the goal of the function, module, etc, and the "conceptual model" behind it helps a lot too (and this is a kind of comment that is unlikely to get stale, since it is related to the overall architecture). Comments explain the algorithm can be helpful if the algorithm is not trivial and/or to explain some choices that seem strange at first sight (e.g., "We use this structure instead of the more obvious ... because later we need to ...")

 
jeremyf profile image
Jeremy Friesen

I have struggled for find the study/observation that I once read; the study/observations looked at teaching folks to code. They also wanted to minimize the number of guidelines for these new programmers. And they found that with the one guideline/rule of methods length many other best practices began to emerge.

An Animated Guide to Node.js Event Loop

>> Check out this classic DEV post <<