How to fix magic numbers and detect primitive obsession

I recently got a task to implement a product importer for a new vendor for a webshop. It seemed alright, but magic numbers and a magic sequence gave me quite a headache. But how can we avoid it?

How does the system look?

The concept is relatively simple; each vendor provides a CSV file with all the product information. Every day the file is updated with new products, stock, and price information. ​
The previous developers on the project already implemented several product importers for other vendors.
They split the importer into two steps. A vendor-specific part that transformed the CSV file to a common format. And a common import system.


​I like the way they split it up.

  • Each vendor Adapter only has one responsibility and is small.
  • We avoid import duplication because it is centralized in the common importer.

What is the issue?

Fetching data from the CSV file looked like this.

A generator is a good idea since the files can be huge. But I see two huge problems with this method.

  1. There is no indication of what the numbers referer to
  2. The elements’ order is essential, but there is no indication of the right order.

It is easy to get something wrong here.

Fixing the problems

​Magic numbers

The numbers referer to the columns from the CSV files, 0 = column A, 1 = column B, etc.

​Since each vendor’s CSV file has a specific format, we can create constants to map the names into meaningful names.

It is much more readable, and we can more easily verify that our mapping is correct.

​Much has been written on magic numbers.

https://en.wikipedia.org/wiki/Magic_number_(programming)

https://refactoring.com/catalog/replaceMagicLiteral.html

Magic sequences

How do we know that we didn’t mix up the sequence of the columns. It is just an array. It doesn’t force us to have any specific order.

​The issue is that we return a primitive type. If we change it to a real type, we gain several benefits.

  • We can use the IDE hints to see that our sequence match up
  • We can quickly implement validation on the fields where needed – we could also move the strtolower into the CommonProduct class if appropriate, it wasn’t in this case.

The concept is often known as “Primitive Obsession”