1. Commented code
Commented code has no place in your codebase. Version control systems exist for a reason, among other methods of avoiding this.
If you are testing out a piece of new, replacement code in your system, find an alternative way to do it. For example, represent the service with an interface and swap them, while deprecating the former implementation. An alternative is to make some sort of configuration to switch strategies of execution. The point is: there are better ways to remedy this.
If you do not need the code anymore, just remove it completely. The fear of throwing away a piece of dead code just because you are sentimentally bound to it does not quite scream “professional”. Once again, version control systems.
If you are for some reason using this snippet to occasionally display/print/generate some report, commenting the code still does not qualify as a valid solution. If it truly is used that often, make it a service.
These snippets in your codebase are confusing to the reader and will only cause unease and latency when reading. Having commented code in your system shows a lack of care for the codebase.
2. DRYing based on coincidence
WET (we enjoy typing) code is terrible. Unnecessary duplication and redundancy breed a lot of issues. Anything you write is prone to errors, but duplicated code especially so. You now have more work to do: remember to keep all of the “replicas” in sync.
We found a remedy for the WET code. DRY (don’t repeat yourself) the code. Extracting the constants, methods, even services can be a very useful and powerful refactoring tool. But also a very dangerous one. With great power comes great responsibility.
Use it with deliberation. Two or more functions can be syntactically nearly identical but semantically worlds apart from each other. Let alone the constants you have in your code.
If it looks like a duck… it might not be one. Only a duck is a duck, not everything that looks like one. Actively think about the consequences of your actions when refactoring (and when not refactoring).
Extraction rooted in syntax is a form of very bad coupling. In your attempts to improve the code quality, you, unfortunately, made it worse. And the bugs are now even harder to trace.
3. Boolean parameters
A boolean parameter in a function is a code smell. A pretty severe one as well. It often displays one of the following: the laziness of the author engineer, rush to meet the deadline, insufficient understanding of the function in question.
The function clearly performs a certain action if the boolean evaluates to true, and a different one if it evaluates to false. Now, unless the function is named ifParamTrueThanSomethingOr... the name is hiding the logic and it is misleading the reader. If it actually is named this way, run!
Jokes aside, there are no benefits to this approach, only harm. The code is harder to read and refactor if needed. Because you are coupling things together. In most cases where we have additional snippets of logic dependent on the boolean parameter, we have multiple “clients” of the method. These clients obviously have different requirements. Imagine if your system logic changed, you would now need to locate all usages and reassign the appropriate boolean input.
Named parameters remedy these issues, but not sufficiently. They only make it easier to read code, all of the other issues remain. If you are still not convinced, decide for yourself where you will draw the line. How many things will your functions do? Remember each boolean adds a power of two to the possible usages of your function.
4. The one-liner
For some odd reason, young software engineers think writing condensed code is a virtue. Somehow, there is a notion that it makes you look smart and that this type of code is impressive.
Favor clarity over cleverness. Inlining is a great feature, and it has many applications where the code becomes simplified and easier to read and comprehend. But inlining up to a point where a single line of code performs 10 things in an obscure and seemingly magical manner is a school example of a code smell.
No one thinks you are impressive because you can write this type of code. It is not worth it. The code is now so condensed, that if you even have to make the slightest change to it, you are unable to. Just because it makes nearly everyone who reads it feel stupid, it does not imply that the code itself is smart.
The same applies to using odd features of the language you are working with. If it is not a well-known and widely used feature of the language, deeply evaluate it before using it. Your code should make your teammates feel comfortable and confident in what the code is doing, not make them afraid and leave them questioning the validity of it.
5. The three-times rule
If you find yourself writing the same piece of code three or more times, you should consider extracting it to a separate method, and perhaps even make it generic.
Not all duplication is evil, but some truly is and it may cost you. You will have to remember to synchronize all methods if a change is introduced, sometimes even update the corresponding documentation as well.
In all likelihood, you will forget some of these and create an issue incredibly hard to trace. Anyone who has encountered this type of bug knows exactly how difficult it is to find it.
Three times is just an arbitrary measure. The point being made is you should be extremely aware of duplication and what it means for your system. Does it scale well? What is the impact of having duplicated logic in your system? What is the breaking point where you finally decide to refactor this code?
Of course, as always, do not just mindlessly perform this. Evaluate the syntactic and semantic similarity of the usages and extract if the cohesion is strong enough. Do it carefully because you are essentially trading off duplication for binding and coupling. A risky move, sometimes worth playing.
If you made it this far, you might want to check out a book with related topics:
The Missing Semester: 100 things you were supposed to be told about software engineering.
Top comments (0)