loading...
Cover image for Clean Code – Part 1

Clean Code – Part 1

dalbirburhm profile image Dalbir Burhm Updated on ・2 min read

Clean code is akin to keeping your home/workspace tidy. Cleaning requires effort, whereas leaving garbage doesn’t.

While I’d love to mention some examples of poor code I’ve seen over the years, I too have fallen foul in the past.

Clean code isn't about vanity! Large messy un-refactored code only adds to technical debt. When another developer works with the same code you’ll have to double the time for them to complete the task due to messy code. Bear in mind time and effort can quickly grow exponentially as each developer hacks and contributes to the same messy code.

Below are suggestions to help keep our code clean.

Move hard-coded values to the database

Instead of this…
Alt Text
do this...
Alt Text
It’s very easy to write constants such as tax rates and bool switches directly into your code without thinking about the long term repercussions. I’m referring to changing values.

In cases like these it is far better to move these values into a database table, or at the very least the application configuration file. This way it will be much quicker and safer to make changes as opposed to opening up the code many years later and making changes, compiling and then praying its works while you upload… you get the idea.

Guard Clauses

Alt Text

Guard clauses follow the fail-fast principle. In the example above I’m performing a null check on the injected IEmailService reference supplied to the constructor (assume I’m using DI). If the reference is null, a related exception will be thrown during runtime.

Dealing with null references is an act of life for any developer. Using guard clauses throughout your code promotes error checking and is a good habit to develop.

Eagle eyed developers may have noticed I’ve compacted an IF statement to one line. You don’t have to do this, but I find it creates less noise and is more succinct.

Choosing appropriate class names

Poor class names…
Alt Text
Class names typically work best when they are nouns. However in more complex scenarios as shown above it is easy to fall foul of this when you need to describe two related classes.

Here’s one solution to fix this:

Alt Text

This is much cleaner and concise. I’ve used inheritance by creating a base class of Customer and then deriving two subclasses from it. In this instance, it allows for shorter class names since there is some context through the inheritance.

As a final note, I’m not a fan of using inheritance-based models everywhere throughout a solution unless there is a valid case as illustrated above. It can add to unnecessary complexity.

Thanks for reading! 🙂

Part 2 is available here:

Posted on by:

dalbirburhm profile

Dalbir Burhm

@dalbirburhm

Dalbir is a .NET Developer (MCSD certified) based in the East Midlands, UK. Since the early 2000s he’s built web solutions and provided software consultancy to small and large blue-chip enterprises.

Discussion

pic
Editor guide
 

Hi Dalbir,

I'm really undecided on your advice to "Move hard-coded values to the database". As usual, there are pros and cons to each approach:

  • Adding extra database calls introduces extra complexity to the solution, making testing/deployment/recovery of the solution in isolation trickier, and potentially with performance impacts if you are in a performance-critical environment.
  • Knowing which hard-coded values are actually likely to change (TaxRate versus DaysInTheWeek, for example) is the key.
  • In many environments, some of those values might need to be legally auditable (e.g., "what TaxRate was used when you calculated the GrossTotal of X for this customer?"). Having configuration in a database (or anywhere outside the code) makes that auditability hard (but certainly still possible).

I think I would externalise configuration to a file before a database for many of these reasons, especially as I can version control the changes more easily.

 

Agreed. One should also not add a database to their project for only this purpose!

One alternative is having a dedicated constants code file that depends on nothing, so everything else can depend on it.

 

For values subject to change, we've used databases in the past. This way, authorized users could change them without our involvement. The tables were "insert only", with the date and username of insertion. This allowed us to, for example, run reports with the value (e.g., tax rate) at a given date if we needed to (as opposed to the current value). Call it time travel.

Stuff like DaysInTheWeek would be a constant in the code. It makes for more readable code as opposed to an hardcoded value.

 

Yep, that's a very good point - particularly the audit requirement.

 

Great Article!
I've been using the guard clauses a lot in my recent framework.
While mine might look a bit more hacky, it does the same as your code snippet:

public OrderService(IEmailService emailService) {
    _ = emailService ?? throw new ArgumentNullException(nameof(emailService));

    // do rest of code here
}

I've been using that line throughout my code base, just to reduce internal screams. xD

 

Writing code is very much like cooking: you're going to make a mess along the way, e.g., using too many mixing bowls, pots, and spoons, spilling food, and having a few food-burning accidents. A good cook knows he's made a mess and will clean up sooner or later. But, just remember: it's not a crime to take a little time to clean up the mess. And, what the cook calls it "clean", the next may say it is "a pigsty." There are a lot of tools out there to help with the clean up. Use them.