What are the code smells or antipatterns that drive you nuts?

#1

Share your biggest bugaboos, the things that increase your WTFs/minute by an order of magnitude, the things that really get under your skin when it comes to reading and maintaining other people’s code.

0 Likes

#2

We had a big discussion at one of my previous workplaces about whether ternary operators or if statements were better. It came up because, occasionally in code reviews, we had a team member who would change any if/else blocks in our code to ternary statements. Keep in mind, this was all written in Perl, which is a little hard to read compared to some other languages anyway.

I even had one that was something like this, where we were splitting strings into two arrays depending on certain conditions, and wrapping it all in a ternary statement:

my ($arr3, $arr4) = (index($str2, ‘-’) != -1) ? ([split /-/,$str2], [split /;/, $str4]) : ([split /:/,$str2], [split /;/, $str4]);

I think this type of thing is super excessive. Not that it’s wrong. But I think there comes a point when you sacrifice readability for reducing code, which probably isn’t the best solution.

0 Likes

#3

I completely agree. Optimizing for lines of code is terrible on so many levels. It creates these blobs of unreadable random characters. Even worse, it inspires people to use obscure acronyms as variable names so they can cram even more logic into even tinier spaces.

0 Likes

#4

I think each has its place. I personally won’t use a ternary operator if it makes the code less readable, which is often. I’ll use it to replace a simple if statement, though. The example you cited is horrific…Perl is bad enough! I say this as someone who fell in love with programming through Perl.

Fun fact: code golf goes back to the 60s, at least. There was an excellent story in Steven Levy’s Hackers about the “decimal print routine”, an algorithm that would convert binary numbers to decimal on the TX-0. I’d highly recommend reading the book – it was my field guide to programming for years, and I still like to re-read it now and then, to remind myself why I hack. :slight_smile:

At that time, on those behemoth computers, it made sense to shave off CPU cycles like this. These days, unless you’re coding for an embedded system or something, memory and CPU cycles are not so much of a concern. At the end of the day, I think it’s yet another programmer pissing contest, like editor flame wars or “real programmer” arguments. I’ve already seen enough of those for one lifetime. Let’s just hack!

0 Likes

#5

I don’t care too much about code aesthetics (basically anything that a code linter or debugger would raise a warning about) - For me this is minor stuff that can easily be changed later.

My biggest concern is when classes or modules do not have clear and distinct responsibilities. If a developer cannot explain in simple terms what a class or module does (to someone who understands the business domain), then that is a code smell. If a class or module is high-cohesion then every part of it should be related in a straight-forward and obvious way and therefore, it should be easy to explain to a new developer who does not understand the low-level details.

Also, cohesion is often inversely proportional to the degree of coupling between different classes/modules. If classes or modules do not have clear and distinct responsibilities, then over time they tend to overstep into each other’s territory; when this happens, modules/classes end up exposing complex interfaces in order to allow themselves to essentially micromanage each other’s internal states (which is bad design).

The goal of modules and classes is to abstract away from low-level technical details so that different parts of the system can be changed independently of each other and can also be easily moved around without breaking the system.

1 Like

#6

Excellent points, thanks for sharing! Overall, I couldn’t agree more.

I try to keep the Unix philosophy in mind when I code, especially:

Make each program do one thing well.

and

Expect the output of every program to become the input to another, as yet unknown, program.

Please note, when I read “program”, I think “module” or “class” as well. The former is hard enough, but the latter takes an especially trying, conscious effort.

Unfortunately, I’ve had to maintain code like this, and it’s truly the stuff of nightmares. “Micromanaging each other’s internal states” is a particularly horrible code smell for me. It gives me flashbacks to managing distributed timers…but that’s another story. In short, it’s all too easy to create bad abstractions that still work for your use case in that moment.

I agree, this is the ideal, the dream. It raises a couple of interesting questions though:

  1. How do you avoid leaky abstractions?
  2. At what point do you decide to create an abstraction?

I know these are hard questions, but I think they could start an interesting conversation. :slight_smile:

0 Likes

#7

Those are very good questions and it takes a lot thinking on a case-by-case basis to be able to answer them.

That said, as a rule of thumb, I think that it’s wise to avoid inventing new abstractions out of thin air. Ideally, abstractions should be derived from high level concepts which already exist in the real world and are relevant to the business problem that you’re trying to solve.

Good abstractions should free developers from having to think about concrete system-level entities like servers, sockets, databases, message queues, etc…

In my view, an example of a leaky abstraction is one which exposes a mix of concrete system-level entities with higher level ‘business’ concepts.

For example, an online shopping cart is an abstract business concept but the database table which stores the actual rows of products which make the shopping cart work is a low-level system entity.
If there was a ShoppingCart class, then its methods should not expose database pointers to the raw data; that would be a leaky abstraction. In that case, a leaky abstraction is dangerous because it would prevent you from easily changing to the system; for example, if later you decided to change the implementation of the shopping cart to store data in memory, in a cookie or in localStorage… It would be a lot more work if the higher level components which depend on your ShoppingCart also have references to database pointers. It’s important that the ShoppingCart hides these low level details from its dependents.

2 Likes