Eclipse Indigo: static code analysis about static and why you should care

Eclipse Indigo comes with new optional static analysis ‘warnings’ which automatically detect methods that could be static.

Eclipse Indigo 'method could be static'

Eclipse Indigo 'method could be static'

To activate the warnings associated with this feature go to Windows->Preferences->Java->Compiler->Errors/Warnings->Code Style (or just search for ‘warn’, see screenshot) and click on the Ignore drop down box and select Warning. I have not tested the ‘potentially be static’, maybe I will turn it on when there are no warnings left to fix from the first one.

Reasons for the addition of this feature:

Static methods:

  • limit scope to static variables and passed parameters, thus avoid potential side effects
  • do not require an instance to be called
  • may not be part of a specific class and moved to a more appropriate place (tool/utility)
  • increased performance

Static methods are one step towards ‘pure functions’, here is the definition of a pure function: “The only result of invoking a pure function is the return value”. To learn more about pure functions, read this post about pure functions.

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away.

Antoine de Saint-Exupéry (French writer)

Adding the static keyword means removing (reducing) scope and potential mistakes (present and future). Bear in mind that a bunch of static methods are enough in some cases. This is why I am happy about this feature and you should too, so be sure to turn it on if you happen to be using Indigo. Have a look at the other warnings, some interesting ones are ignored by default.

Related:

Advertisements

Why every Java field should have been final by default.

Introduction

Note to clarify the post: by default I mean having to write something like ‘var’ to have the current behavior (fields can be reassigned), so that if you write nothing the field is automatically final. You still have the choice about being able to reassign or not by using some kind of keyword like ‘var’. I hope it is clearer now !

We all like freedom, but what is freedom when we talk about code ?

You have a lot of freedom with mutable fields, too much, why not let the compiler help you by telling it that your data is immutable ? Is it because you want more possibilities to write code that does not do what you want ?

You sure do not want to, and please do not tell me the final keywords make your code hard to read, it does the exact opposite: it tells the compiler and the reader that your data will never change and this is important information. Most of the time it forces you to write cleaner code that uses less ‘smart tricks’.

(Note: this post is NOT about the ‘other’ final keyword, the one used on classes and methods. It is also not about immutability in general, a much broader topic, but the given links will provide some lecture.)

Examples that illustrate why every Java field should have been final by default:

Example 1: Bad initialization in constructors.

public class UnidentifiedPerson {

 private String _firstName, _lastName;

 public UnidentifiedPerson(String firstName, String lastName) {
   _firstName = firstName;
   // Notice how the lastName is never set and the compiler does not complain !!!
 }
}

There is rarely a need for such code, most of the time you will find that making the fields final will force you to write better constructors.
This is especially true when you have many fields to initialize and when you refactor code.

Example 2: Changing data that is immutable by nature.

public class Person {

 private String _firstName, _lastName;

 public Person(String firstName, String lastName) {
   _firstName = firstName;
   _lastName = lastName;
 }

 public Person changeIdentity(String firstName, String lastName) {
   // It is so easy to change my name.
   _firstName = firstName;
   _lastName = lastName;
   return this;
 }

 @Override
 public String toString() {
   return _firstName + " " + _lastName;
 }

 public static void main(String[] args) {
   System.out.println(new Person("Christophe", "Roussy").changeIdentity("Bill", "Gates"));
 }
}

If you make the fields final the compiler will tell you something interesting:

public PersonFinal changeIdentity(String firstName, String lastName) {
 // The final field PersonFinal._firstName cannot be assigned.
 _firstName = firstName;
 // The final field PersonFinal._lastName cannot be assigned.
 _lastName = lastName;
 return this;
 }

Example 3: Hiding potentially useless code.

public class UselessAssignment {
 public static void main(String[] args) {
 int a, b;
 a = 1;
 b = a;
 a = b;
 // This kind of code is only possible with mutable fields, it will
 // eventually turn up because it can.

 final int fa, fb;
 fa = 1;
 fb = fa;
 fa = fb;
 // The final local variable fa may already have been assigned.
 }
}

When prefixing fields with the final keyword, some undesired code will emerge, it may even explain and solve some bugs.

When not to use final ?

There a only few cases where you want to use mutable fields, such as computing sums:

private static int computeSum(final List<Integer> values) {
   // Here it makes sense not to use final as you do WANT to change the value
   // of sum. Also note how I now use int for sum to avoid autoboxing.
   int sum = 0;
   for (final int value : values) {
     sum += value;
   }
   return sum;
 }

A good way to figure out if you want to remove the final keyword is to ask yourself: “Do I really want this variable to be mutable ?”.

It can sometimes be tricky not to use mutable fields when you are used to them, but on the long term you will discover that it is well worth the effort to help the compiler so it can help you and others. Also note that making a list final does not make the list fully immutable, you will not be able to reassign the list itself, but you will be able to add, remove and modify the contained elements. I have also read that the value of final fields can be changed using reflection (http://www.javaspecialists.eu/archive/Issue096.html), but this is not a concern unless you want to experiment.

Conclusion

It is illogical that the Java language did not do the opposite: every field immutable by default and mutable only when prefixed with some language keyword. Perhaps it comes from the C/C++ mutable flavour Java inherited.

Links:

  1. http://renaud.waldura.com/doc/java/final-keyword.shtml, The Final Word On the final Keyword
  2. http://jeremymanson.blogspot.com/2008/04/immutability-in-java.html, Immutability in Java.
  3. http://www.ibm.com/developerworks/java/library/j-jtp1029.html, Is that your final answer?
  4. http://www.ibm.com/developerworks/java/library/j-jtp02183.html, To mutate or not to mutate?

Composition versus Inheritance

In many cases you should favor composition over inheritance, I was willing to write an article about this, but I prefer to redirect you to an existing article:

Composition versus Inheritance
A Comparative Look at Two Fundamental Ways to Relate Classes
by Bill Venners

http://www.artima.com/designtechniques/compoinh.html

I recommend reading other articles on this site because they mostly address fundamental design issues:

http://www.artima.com/lejava

Work towards the compiler

This is the first post !

You may think that you know what you are doing because it is your code after all, but if a mistake can be done it will be done (when refactoring or by someone else modifying your code).

This post illustrates how easy it is to write error prone code, but also how little effort it may take to reduce the danger.

Equals method:

Consider the following code sample:

package javarizon;

import java.util.ArrayList;
import java.util.List;

public class TypeSafeEquals {
 public static void main(String[] args) {
 final String one = "1";
 final int oneAsInt = 1;
 final Object object = "1";

 // Compiles...
 one.equals(oneAsInt);

 // First idea: It works, but if you pass an object by mistake it still
 // compiles. Have fun debugging !
 one.equals(String.valueOf(oneAsInt));
 one.equals(String.valueOf(object));

 // Better idea: It works, but will not compile if you use an object.
 one.equals(Integer.valueOf(oneAsInt).toString());
 one.equals(Integer.valueOf(object).toString());

 // Even better: it only takes an int.
 one.equals(Integer.toString(oneAsInt));
}
}

The first version is often used because it easy to come up with, but if you accidentally use a bad parameter, it will compile and not even break at runtime: it results in a bug that will be difficult to spot.

It might even work by chance in some cases (there are only two possible return values) depending on your application logic and data. Imagine you use such non-type safe code everywhere in your code, you or someone using your code is doomed to make a mistake at some point.

In the second version, the chances of committing a mistake were drastically reduced simply by using code that will accept less types, it does not compile when passed an object:

The method valueOf(String) in the type Integer is not applicable for the arguments (Object)

Consider using code completion on a method to quickly check what types it can take, if there are too many (especially evil Object) try looking for a more restrictive alternative. You could also write a static method that takes two Strings and uses equals internally, the only problem is that you have to do this for every possible type.

Conclusion:

Being careful is not enough, when properly helped the compiler is much more careful than you ever will. Always strive to maximize type-safety when writing code. Ensure the compiler is on your side by adapting your programming style to it.

I will try coming up with more examples in the future posts.