Clean Code

When I review candidate code test submissions, naming is the first thing I look at. Too many engineers with Senior titles use poorly named variables and methods. To me, this failure suggests they understand how the program should run but would struggle in a larger codebase with multiple developers.

Most programmers fail naming in a few ways:

1. Ambiguity. What does “analyzeData” actually do?
2. Fear of verbosity. getPriceAfterDiscountApplied gives a lot of developers the creeps, but it’s crystal clear what that method does. As I progress in my career, I favor more verbose and explicit names.
3. Unnecessary shortcuts. For some reason, developers love to cut vowels out of variable names. Stop it.
4. Inconsistent prefixes and suffixes. In the same code sample, I’ve seen a user, userObj, and userObject. I have no idea what the difference is. Related to this is the inconsistent use of hungarian notation vs. not.

Improving your naming skills is not difficult, but it does require diligence and practice. In turn, you gain freedom.

Rule of thumb, methods should be about 10 lines or shorter. Larger methods are generally a code smell that your method:

1. Does too many things
2. Doesn’t have adequate separation of concerns
3. Is difficult to test

Learning to decompose methods is probably for Advanced Beginner to Competent developers, per the Dreyfus Model.

This one took me a long time to really understand, particularly given my experience in PHP where the core language libraries often have functions with 3-4 parameters.

To me, it felt like additional parameters provided flexibility. I didn’t understand initially how to push that flexibility to a higher layer.

Fewer params make your code more testable with fewer paths to test per method. Fewer params means more methods, but more testable methods.

As with most code readability improvements, there exists high correlation with other advice like small functions and not passing null.

Returning and passing null results in a lot of unnecessary guard clauses.

Particularly for PHP developers, removing null passes and returns will feel foreign. We’ve been trained to pass null around and never fail the user.

But often, throwing an exception is better than passing null. It results in cleaner code that fails, but you have an opportunity to know about those failures. And you don’t need to account for the possibility of null in every single caller and callee.

Sometimes I get code test submissions where every line is commented. That’s an obvious mistake.

More subtle issues come with docblocks and other explanatory comments.

Each explanatory comment is an opportunity for discrepancy between the actual code and comment. Engineers update code to fix bugs, but we often don’t revise the comment.

In most cases, you’re better off just deleting the comment. I’d rather have no comment than anything outdated or inaccurate.