Why I Won’t Accept ANY Magic Number

One of the first things I like to establish in a new project is the use of tools like Checkstyle and Findbugs in order to codify some code guidelines and to avoid bugs that can be determined by static code analysis.

Sooner or later with these tools one stumbles over a case where people have the feeling it is going to far. One such case is the check for Magic Numbers of Checkstyle. It will warn about any use of numeric literals except -1, 0, 1 and 2 that aren’t used to define a constant.

Many developers have a problem with this check as one can see from the resulting code. I have seen code like this

  1. private static final int FOUR = 4;

and also

  1. private static final int FOUR = 5;

and my all time favorite (I’m not making this up!):

  1. firstname = rs.getString(1);
  2. lastname = rs.getString(2);
  3. city = rs.getString(2 + 1);
  4. zip = rs.getString(2 + 2);
  5. country = rs.getString(2 + 2 + 1);

But there is a different case where the discussion comes up. It’s with obvious constants like 100 for converting fractions into percent values or 1024 for conversion between bytes and kibibytes. Some argue these aren’t magic numbers (or magic numbers but not as bad) because their meaning is obvious and they won’t change.

I disagree and will vote to make them constants any time. Here is why:

1. The meaning is NOT obvious. What does value * 100 mean? Is a fraction converted to percent? Is a length measured in meters, converted to centimeters? Or something multiplied by a rough approximation of g*g with g being the gravitational acceleration on earth? Or maybe I’m multiplying something with the size of an array, that happens to be 100. Can’t tell. A simple constant with a proper name will fix that.

2. Ok, I do agree, many of these constants won’t change. But the purpose of defining constants (or methods, or classes) is not (only) to enable later change, but to make reading, understanding and reasoning more easy. So the question if a value might change in the future is completely irrelevant.

3. (This one is the argument that I’m missing in most of the discussions) I just don’t want to think about it or have others think about it. I have seen dozens, probably hundreds of instances, where a properly named constant would have helped tremendously understanding a piece of code. And I have seen very few cases where it might have hurt readability and not a single one where it hurt badly. Therefore: just extract a constant and be done with it.

Note: Just because it is a constant doesn’t mean it has to be public, or even a class level field. If it is used only in a single method a local variable is just fine.

What I look for in frameworks

In every project the discussion comes up over and over again: should we use framework X? or Y? or no framework at all? Even when you limit yourself to the frameworks for web development in the Java space the choices are so plentiful, nobody can know them all. So I need a quick way do identify which frameworks sound promising to me and which I keep for weekend projects.

  1. Stay away from the new kid on the block. While it might be fun to play with the coolest newest thing, I work on projects that have a life cycle of 10-30 years. I wouldn’t want to support an application using some library that was cool between March and July in 1996. Therefore I try not to put that kind of burden on others.
  2. Do one thing and do it well. A bad example for this is Hibernate/JPA. It does (or tries to) provide all of the following

    • mapping between a relational and an object-oriented model
    • caching
    • change detection
    • caching
    • query dsl

    It is kind of ok for a framework or library to provide multiple services, if you can decide on each service separately if you want to use it or not. But if it controls to many aspects of your project, the chance that it doesn’t do anything well gets huge. And you won’t be able to exchange it easily, because now you have to replace half a dozen libraries at once.

  3. Method calls are cool. Annotations are ok. Byte code manipulation is scary. Code generation a reason to run for the hills. In the list only method calls can be abstracted over properly. All the other stuff tends to get in your way. Annotations are kind of harmless, but it is easy to get in situations where you have more annotations than actual code. Byte code manipulation starts to put some serious constraints on what you can do in your code. And code generation additional slows down your build process.
  4. Keep the fingers of my domain model. The domain model is really the important part of an application. I can change the persistence or the ui of an application, but if I have to rework the domain model, everything changes and I’m essential rewriting the application. Also I need all the flexibility the programming language of choice offers to design the domain model. I don’t want to get restricted by some stupid framework that requires default constructors or getters and setters for all fields.
  5. Can we handle it? There are many things that sound really awesome, but they require a so different style of coding, that many developers will have a hard time tackling it. And just because I think I can handle it, doesn’t necessarily mean I actually can. So better stay simple and old-fashioned.

The Latest Version of Degraph is Available for Download at Maven Central

Degraph is my personal open source project for visualizing and testing package dependencies in JVM byte code. I released the latest version 0.1.1, so go grab it while it is hot.

What’s new

This release in mainly a bug fixing release. So the important changes are two bug fixes. Degraph 0.1.0 missed a few dependencies to static methods, and through class valued annotations. So you absolutely should upgrade. I hope you don’t use many static methods, so the first bug won’t bite you as bad. The second one is actually quite annoying with certain frameworks. Spring for example uses lot’s of annotations, including those that contain references to classes.

Fortunately upgrading your tests will be easy, because degraph-check is now available through Maven Central, which makes using it really a no-brainer.

Although this release is just a minor one, a tiny new feature slipped in almost by accident. And when I tried it at work it turned out to be extremely helpful: You can now instruct Degraph to create a graphml file for failed tests. Just provide a path using the printTo method. An example looks like this:

  1.   test("Check identifies cycles in junit") {
  2.     classpath.printTo("junitDependencyFailure.graphml").including("**.junit.**") should  not be (violationFree)
  3.   }

This allows for a relative quick workflow:

  • Write your test
  • Run the test
  • Open the resulting graphml file in yed
  • Modify code or test
  • Run the test
  • Reopen in yed
  • Repeat the previous three steps until done

I found this tremendously helpful when fixing the problems that the bugfixes mentioned before surfaced.

Give it a try and let me know about your experiences in the comments below, or in the issue tracker over at github.

Softwaredevelopment, Learning, Qualitymanagement and all things "schauderhaft"