According to Wikipedia, Code smell, also known as bad smell, in computer programming code, refers to any symptom in the source code of the program that possibly indicates a deeper problem. Yes, nothing tangible presented here. Just the smell. Bad smell

Before digging into this article, I have a question for you: "Are CSS difficult?".

Most of the developers I have encoutered in my life so far tend to have a common answer: "It should be like a walk in the park". Yeah, more like "a swamp park". CSS might seems easy at first but it is indeed quite complicated, because there are so many ways to do things in CSS and different developers do it differently. Some of those are very sloppy that they apply a bunch of CSS styles and make the layout to work without being aware of why it works and how long it will stay working. Putting it together, it is really confusing. Thus today, I bring you some patterns in CSS considered CSS code smells that I have run into in my frontend career until now.

1. Style undo/Style takeaway

Any CSS that unsets styles (apart from those in a reset.css) should start ringing alarm bells immediately. The very nature of CSS (Cascading Style Sheet) is that things cascade and inherit from those defined previous. Rulesets should only inherit and add to the previous ones, never undo.

h3 {
  font-size: 2em;
  margin-bottom: .5em;
  padding-bottom: .5em;
  border-bottom: 1px solid #ddd;
}

The example gives us all h3s our usual font-size and margin for spacing, but also a bit of padding and border-bottom in order to visually separate it from the next element on the same page. There is nothing "morally wrong", it is just that you might have probably applied them too early and hastily. What if there is a circumstance that we need the h3 but without that border-bottom and padding-bottom?

Piece of cake, we can achieve such thing by the follow way:

h3 {
    font-size: 2em;
    margin-bottom: .5em;
    padding-bottom: .5em;
    border-bottom: 1px solid #ddd;
}

.no-border {
    padding-bottom: 0;
    border-bottom: none;
}

NO. That has totally gone wrong. What did we really achieve in the end? A modified version of h3 with no border and padding? More like 10 lines of CSS and one super ugly class name.

What might have been better is the version below:

h3 {
    font-size: 2em;
    margin-bottom: .5em;
}

.paragraph-heading {
    padding-bottom: .5em;
    border-bottom: 1px solid #ccc;
}

Eight lines. Not drastically shorter but containing no style undo and a sensible class name. Everyone is happy.

As you go down a stylesheet you should only ever be adding styles, not taking away. If you find out that the more you go down your document, the more frequency you have to undo styles, well, the chances are you jumped the gun and started adding too much too soon. Imagine CSS like this over tens of thousands of lines… that’s a lot of unnecessary undo. Break components into simpler components that came before it. Do not start too complex and risk having to undo your work later on; you’ll end up writing more CSS to achieve less styling.

2. Magic numbers

Magic numbers are the ones that are used ‘because it just works’. A kind of bugbear which often presents in a lot in projects. Look at the following piece of scss code:

.main-nav {
  // some styles
  
  li:hover {
    .dropdown {
      @include absolute($top: 39px, $left: 0);
    }
  }
}

We have a navigation menu, some of them include dropdown when hovering. The .dropdown flyout menu needs to appear at the bottom of the navigation. Here, $top: 39px is a magic number. The only reason it works, presumably, is because the lis inside .site-nav happen to be 39px tall. It is entirely circumstantial and thus, we should place no faith in it. What if someone changes the font-size in .main-nav and now everything is 27px tall? This number is no longer valid and the next developer needs to be aware of this change and update it.

And don't forget the problem of cross browser. What if Chrome does render the lis at 39px, but IE renders it at 38px? That number only works in one situation... We would be far better off replacing $top: 39px with $top: 100%, which basically means ‘all the way from the top’.

Magic numbers have several problems associated with them. As mentioned, they cannot be relied upon, but also, with their very ‘just because it works’ nature, it’s difficult to communicate with another developer about that number's origin. When the magic number becomes invalid, you might face one or more of the following problems:

• The next developer might not understand a damn thing where the magic number came from, so they delete it.
• The next developer who is more cautious decides to try and fix the problem without touching that magic number. This means that an old, outdated, hacky magic number stays in the code, and the next developer simply hacks away on top of it. You are now hacking on top of a hack. Congratulations!

No sooner or later, magic numbers will become out of date. They not only does confuse other developers, but also can neither be explained nor trusted her developers. There’s nothing worse than hitting someone else’s code and seeing an inexplicable number. You’re left wondering what the hell it does, why it’s needed and whether or not you should dare touch it.

In summary, please avoid magic numbers like the plague.

3. Qualified selectors

Normally when you try to qualify something, it is often for good purpose. But sometimes, you action might cause catastrophic consequences. Like these:

ul.nav { }
a.button { }
div.header { }

Basically, selectors which are needlessly prepended by an element. These are also bad news because they:

• Totally inhibit reusability on another element
• Increase specificity
• Increase browser workload, obviously decreasing performance.

Why make a browser look for a class .button on an a when you could just ask it to look for .button and be done? By qualifying selectors you are increasing a browser’s workload. By not doing so, you can:

• Save actual amounts of code
• Increase performance
• Allow greater portability
• Reduce specificity

Therefore, these bad traits should be corrected like below:

.nav { }
.button { }
.header { }

Now .nav can also be applied to an ol, .button can be applied to an input. Besides, when the site gets ported over to HTML5 - the header div can be quickly swapped out with a header element without us worrying without worrying about invalidating any styles.

4. Hard-coded/absolute values

Not unlike magic numbers, hard-coded values are also bad news. A hard-coded value might be something like this:

h2 {
  font-size: 24px;
  line-height: 32px;
}

line-height: 32px;. Not cool. It should be line-height: 1.333 instead. Setting line-height relative/unitless makes them more forgiving and flexible. Keeping the hard code line-height's value, if you change the font-size, you will have also add new line-height style in order to correspond with the new font-size.

/* Fixed line-height */
h2 {
  font-size: 24px;
  line-height: 32px;
}

.article-title {
  font-size: 36px;
  line-height: 48px;
}

/* Relative line-height */
h2 {
  font-size: 24px;
  line-height: 1.333;
}

.article-title {
  font-size: 36px;
}

Hard-coded values are not very future proof, flexible or forgiving, and thus should be avoided. The only things that should ever really have hard-coded values are things like sprites which will always need to be a certain size no matter what.

5. Brute forcing

This one is in a similar vein to hard-coded numbers, but a little more specific. Brute forcing CSS is when you combine hard-coded magic numbers and a wide variety of other techniques to force a layout to work. Take a look at the example:

.foo {
  margin-top: -12px;
  position: relative;
  z-index: 1000;
  height: 48px;
  float: left;
}

This is terrible CSS and somehow it does not make sense at all when the next developer glance at the code the first time. All of these heavy-handed, brute-forced, layout-affecting CSS are only used to force something to render as and where it is wanted. This type of CSS indicates that either a poorly-coded layout that requires this kind of manipulation or the developers lack of understanding about box-model and layout. In my entire front-end career, such patterns are exactly the "bite marks" of some former backend developers that handled the HTML/CSS tasks previously.

Coded layouts should never need brute-forcing. Empower yourself with a solid understanding of box model, layout and review your code oftenly so as to avoid winding up in such situations.

6. Dangerous selectors

A dangerous selector is one with far too broad a reach. A really obvious and simple example of dangerous selector might be:

div {
  background-color: #ffc;
  padding: .5em;
}

This will instantly scream at any developer. Why on earth would you want to carpet bomb every div on your site? Why would anyone ever want to have a selector like aisde { } for example? Or header { }, or ul { }? Selectors like theses are way, way to far reaching and will ultimately lead to undoing CSS, as per the section previously.

Let's look at the header { } example more closely...

A lot of people use a header element to mark up their site's main top header - which is totally fine - hoever, if you style that side-wide header like this:

header {
  padding: 1em;
  background-color: #dfdfdf;
  color: #fff;
  margin-bottom: 20px;
}

...the it is not that fine. The header element does not bear the meaning of 'your site's main header' and, as per the specification, the header element can be used multiple times in multiple contexts. This should be targeted via a selector more like .site-header { }, for instance.

To give such specific styling to such a generic selector is dangerous. Your styles will leak out into areas they are not supposed to. Moreover, as soon as you start trying to use that element again, you will inevitably need to undo styles (adding more code to take previous styles away) in order to combat this foe.

7. Reactive !important

!important is fine. It is fine and it is an important tool, however, should only be used in certain circumstances.

!important should only ever be used proactively, not reactively.

By this mean, there are times when you know that a certain style will always take precedence, and you will know this up front.

For example, you have analyzed the document specification, discussed with the your Business Analyst along with your clients and finally, reached a consensus that the error message is display in red color. In another word, you know that for the rest of the lifetime of your project, you will always want error message to be red. So this rule is totally fine:

  .error-text {
    color: #c00 !important;
  }

If the error occurs in a div where the text is always blue, we can be confident that we want to break that rule in the case of errors. We always want errors to be red because it’s an error, and user messaging should always remain consistent. Here we can proactively add !important because we know we always want errors to be red.

Where !important is bad is when it is used reactively, that is to say, it’s been used to get someone out of a specificity problem, or they’re in a bit of a bind and resort to !important to force things to work. This is using !important reactively and this is bad news.

Using !important reactively is just a way of circumventing the problems caused by ill-formed CSS. It doesn’t fix any problems, it only fixes the symptoms. The problems still exist, but now with and added layer of super-specificity that will take yet more specificity to overcome.

8. ID buddies

IDs in CSS are a bad idea because they heightened specificity and are of no use to anyone. It is said that IDs in HTML can be used for fragment identifiers and JS hooks but never in CSS because:

• IDs can never be used more than once in a page, which significantly reduces reusability while classes can exist only once or a million times.
• IDs can often have their traits abstracted out into many reusable classes (for example, instead of #success-notification, we might divided into .notification.notification-success or .notification.notification-error with .notification serving as the abstract class).
• An ID is 255 times more specific than one class… infinitely more specific than a class.
• This means you’d need 256 chained classes to override one ID no amount of chained classes can override an ID.

9. Loose class names

A loose class name is one that is not specific enough to define what is its purpose. For example, imagine a class named .board. What on earth does this do? Such class name is bad and here are the reasons why:

• You can't necessarily glean its purpose based on the class name alone. What does it style? Is it a notification board like Facebook? Or is it just a simple board like a panel for displaying content on the page? Does it happen to refer to the Kanban-board we often see during software development process? It it is, then a clearer name like .kanban-board should shove away all misconceptions. A lot longer? Yes. A lot better? Hell yes!

• It is so vague that it might be (accidentally) redefined/reassigned by another developer in a near future. Let's say you are working on an task management site using .board again, and it refers to the kanban-board. Now new requirements arrive, another developer has to implement a notification board in another page to notify users of which tasks they have been assigned. Their temptation might be to use .board again somewhere, which is wrong, but in certain (albeit unlikely events) this could lead to your .board class being redefined and overwritten.

All this can be avoided by using much stricter class names. Classes like .board and .user and suchlike are far too loose, making them hard to quickly understand, and easy to accidentally reuse/override.

Class names should be as specific as possible.