Best Practises – Avoiding Code Smells

Code smell, if you haven’t heard that term before then you probably thought that the title of this article is just bad English. But ‘code smell’ is an actual term in the programming world, and it’s one you should become familiar with.

A code smell, simply put, is something in code that isn’t quite right and could be a surface indication to a much deeper problem in your code. While a code smell doesn’t mean your code is technically wrong, as the code may still function and the system performs correctly, think of a code smell as a weakness in the code. A section of code that doesn’t necessarily conform to good design principles or best practises.

So while it may not mean your code is buggy (yet), it’s still best to minimise code smell where possible. Within the context of test automation, there are a number of areas that usually fall foul of this particular problem and result in the whiffiest of smells.

Code Duplication

First up, and the most common one I see, is duplicate code, particularly when it comes to tests. It’s easy to fall into the trap of writing duplicate code, especially in areas where you’re performing the same steps such as logging in to a system. These steps can’t be avoided, but if you find yourself writing the same code in multiple tests, then this is a code smell.

The biggest problem with this is that while it may work now and be bug-free, if the system changes, you have to make the change to that code in every test you’ve repeated these steps.

The simple fix for this particular smell is to add a layer of abstraction. Create a method in a helper class that deals with this particular set of steps. All these tests that previously had the duplicate code can now call a single method, which also means just a single point of failure if the system changes.

Poor Locators

We’ve all been there, that element you want to use has no ID and is nested so far into the DOM that the XPath or CSS selector is just horrible, but you’re not in the mood to create a relative path so you simply use ‘Copy XPath’ from DevTools and paste it in the hope that it never changes.

Again, this may work perfectly fine at the time, and if you’re lucky, for a while after that. But eventually, it will break, it’s just a matter of time. Using absolute paths and being lazy with your locators will only come back to bite you later on, and it’s one of the worst code smells.

Take the time to write solid relative locator paths initially and you will reap the rewards later on in terms of reliability and low maintenance. Of course, if ID or Name is ever available, just use those.

Long Class or Methods

When writing a new class, or even adding code to an existing class, it can be easy to forget the responsibility or the role of that class. What started off as a small, well-designed class can quickly grow into something that spans hundreds of lines of code. This is another bad code smell.

When a class becomes too big, it’s difficult to understand what the purpose of that class is and to follow exactly what is contained within that class. If a class is that long, then it probably contains more than one responsibility, and if you feel it doesn’t, then that responsibility should be reevaluated.

This same logic applies to methods as well. A method should do only one thing. So if your method is doing something with data, taking that data and doing something else with it, then performing an action based on that other thing, then this is bad. That method should be split into multiple methods, with one for each action that is being performed.

The exception to the rule is when a method is only doing one thing but is still considered long. This isn’t ideal but as long as the method is definitely only doing one thing and has one responsibility, it’s probably ok to leave it.

Bad Waits

In a previous article, I discussed just how bad waits can be if used incorrectly and the difference between implicit and explicit waits. But it is so important to stress again, as the use of bad or inefficient waits is one of my least favourite code smells.

It’s easy to be tempted to put a Thread.Sleep or another type of implicit wait in your code, especially if you think it’s a one-off or inavoidable. Trouble is, it’s never just a one-off as you’ll do it again or they’ll be another similar wait elsewhere in the code.

These waits slow down your tests and if not kept in check, can add huge amounts of time on to the total runtime of your test suite.

Practise how to write good explicit or conditional waits that will not only produce far better and far more readable code but mean your tests aren’t waiting unnecessarily and slowing everything down

Incorrect Use of Asserts

I see this one far too often, and while it may not seem obvious, and in fact, may seem like it’s being helpful, it’s definitely a code smell.

If you’re writing methods in your helper classes, or worse your framework code that includes assertions, then your code hasn’t been well thought out. The framework shouldn’t be responsible for passing or failing a test, it is the job of the test itself to determine the result. Having multiple points of failure across several layers of abstraction can make it an absolute nightmare to debug, or to read results in reports generated by your CI/CD platform.

If you feel a method is suited to having an assert, reassess what it is that the test that is using that method is actually doing, or allow the method to instead determine the state of play and return something, which the test can use to perform an assertion. But never allow assertions to go above the level of the test, not if you don’t want to cause an almighty pong with your code.

So, there are five of the most common areas in the context of test automation that usually have smelly code. If your code is guilty of one of the above, or all of them, don’t be disheartened. As mentioned at the start of the post, having a code smell doesn’t mean your code has bugs in it, far from it. Your code may perform flawlessly for years even with numerous code smells in, but it’s important to understand and be able to identify what a code smell is so that you can improve your code and refactor where necessary.

Make a point of trying to sniff out these smells when you get a quiet period, spotting them early can save you a lot of pain later on when the problem can become very deeply rooted into your code.