views:

596

answers:

8

I understand that in this code:

class Foo {
    public static void method() {
        System.out.println("in Foo");
    }
} 

class Bar extends Foo {
    public static void method() {
        System.out.println("in Bar");
    }
}

.. the static method in Bar 'hides' the static method declared in Foo, as opposed to overriding it in the polymorphism sense.

class Test {
    public static void main(String[] args) {
        Foo.method();
        Bar.method();
    }
}

...will output:

in Foo
in Bar

Re-defining method() as final in Foo will disable the ability for Bar to hide it, and re-running main() will output:

in Foo
in Foo

(Edit: Compilation fails when you mark the method as final, and only runs again when I remove Bar.method())

Is it considered bad practice to declare static methods as final, if it stops subclasses from intentionally or inadvertantly re-defining the method?

(this is a good explanation of what the behaviour of using final is..)

+11  A: 

I don't consider it's bad practice to mark a static method as final.

As you found out, final will prevent the method from being hidden by subclasses which is very good news imho.

I'm quite surprised by your statement:

Re-defining method() as final in Foo will disable the ability for Bar to hide it, and re-running main() will output:

in Foo
in Foo

No, marking the method as final in Foo will prevent Bar from compiling. At least in Eclipse I'm getting:

Exception in thread "main" java.lang.Error: Unresolved compilation problem: Cannot override the final method from Foo

Also, I think people should always invoke static method qualifying them with the class name even within the class itself:

class Foo
{
  private static final void foo()
  {
    System.out.println("hollywood!");
  }

  public Foo()
  {
    foo();      // both compile
    Foo.foo();  // but I prefer this one
  }
}
Gregory Pakosz
+1  A: 

Usually with utility classes - classes with only static methods - it is undesirable to use inheritence. for this reason you may want to define the class as final to prevent other classes extending it. This would negate putting final modifiers on your utility class methods.

mR_fr0g
Good point - one of my refactoring goals is to move static methods to a utility class, marking that class as final is a good idea.
brass-kazoo
+1  A: 

It might be a good thing to mark static methods as final, particularly if you are developing a framework that you expect others to extend. That way your users won't inadvertently end up hiding your static methods in their classes. But if you are developing a framework you might want to avoid using static methods to begin with.

Aditya
+1  A: 

The code does not compile:

Test.java:8: method() in Bar cannot override method() in Foo; overridden method is static final public static void method() {

The message is misleading since a static method can, by definition, never be overridden.

I do the following when coding (not 100% all the time, but nothing here is "wrong":

(The first set of "rules" are done for most things - some special cases are covered after)

  1. create an interface
  2. create an abstract class that implements the interface
  3. create concrete classes that extend the abstract class
  4. create concrete classes that implements the interface but do not extend the abstract class
  5. always, if possible, make all variables/constants/parameters of the interface

Since an interface cannot have static methods you don't wind up with the issue. If you are going to make static methods in the abstract class or concrete classes they must be private, then there is no way to try to override them.

Special cases:

Utility classes (classes with all static methods):

  1. declare the class as final
  2. give it a private constructor to prevent accidental creation

If you want to have a static method in a concrete or abstract class that is not private you probably want to instead create a utility class instead.

Value classes (a class that is very specialized to essentially hold data, like java.awt.Point where it is pretty much holding x and y values):

  1. no need to create an interface
  2. no need to create an abstract class
  3. class should be final
  4. non-private static methods are OK, especially for construction as you may want to perform caching.

If you follow the above advice you will wind up with pretty flexible code that also has fairly clean separation of responsibilities.

An example value class is this Location class:

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;


public final class Location
    implements Comparable<Location>
{
    // should really use weak references here to help out with garbage collection
    private static final Map<Integer, Map<Integer, Location>> locations;

    private final int row;    
    private final int col;

    static
    {
        locations = new HashMap<Integer, Map<Integer, Location>>();
    }

    private Location(final int r,
                     final int c)
    {
        if(r < 0)
        {
            throw new IllegalArgumentException("r must be >= 0, was: " + r);
        }

        if(c < 0)
        {
            throw new IllegalArgumentException("c must be >= 0, was: " + c);
        }

        row = r;
        col = c;
    }

    public int getRow()
    {
        return (row);
    }

    public int getCol()
    {
        return (col);
    }

    // this ensures that only one location is created for each row/col pair... could not
    // do that if the constructor was not private.
    public static Location fromRowCol(final int row,
                                      final int col)
    {
        Location               location;
        Map<Integer, Location> forRow;

        if(row < 0)
        {
            throw new IllegalArgumentException("row must be >= 0, was: " + row);
        }

        if(col < 0)
        {
            throw new IllegalArgumentException("col must be >= 0, was: " + col);
        }

        forRow = locations.get(row);

        if(forRow == null)
        {
            forRow = new HashMap<Integer, Location>(col);
            locations.put(row, forRow);
        }

        location = forRow.get(col);

        if(location == null)
        {
            location = new Location(row, col);
            forRow.put(col, location);
        }

        return (location);
    }

    private static void ensureCapacity(final List<?> list,
                                       final int     size)
    {
        while(list.size() <= size)
        {
            list.add(null);
        }
    }

    @Override
    public int hashCode()
    {
        // should think up a better way to do this...
        return (row * col);
    }

    @Override
    public boolean equals(final Object obj)
    {
        final Location other;

        if(obj == null)
        {
            return false;
        }

        if(getClass() != obj.getClass())
        {
            return false;
        }

        other = (Location)obj;

        if(row != other.row)
        {
            return false;
        }

        if(col != other.col)
        {
            return false;
        }

        return true;
    }

    @Override
    public String toString()
    {
        return ("[" + row + ", " + col + "]");
    }

    public int compareTo(final Location other)
    {
        final int val;

        if(row == other.row)
        {
            val = col - other.col;
        }
        else
        {
            val = row - other.row;
        }

        return (val);
    }
}
TofuBeer
A: 

I encountered one detriment to using final methods using Spring's AOP and MVC. I was trying to use spring's AOP put in security hooks around one of the methods in the AbstractFormController which was declared final. I think spring was using the bcel library for injection in classes and there was some limitation there.

BillMan
A: 

When I create pure utility classes, I declare then with a private constructor so they cannot be extended. When creating normal classes, I declare my methods static if they are not using any of the class instance variables (or, in some cases, even if they were, I would pass the arguments in the method and make it static, it's easier to see what the method is doing). These methods are declared static but are also private - they are there just to avoid code duplication or to make the code easier to understand.

That being said, I don't remember running into the case where you have a class that has public static methods and that can/ should be extended. But, based on what was reported here, I would declare its static methods final.

Ravi Wallau
+1  A: 

If I have a public static method, then it's often already located in a so-called utility class with only static methods. Self-explaining examples are StringUtil, SqlUtil, IOUtil, etcetera. Those utility classes are by itselves already declared final and supplied with a private constructor. E.g.

public final class SomeUtil {

    private SomeUtil() {
        // Hide c'tor.
    }

    public static SomeObject doSomething(SomeObject argument1) {
        // ...
    }

    public static SomeObject doSomethingElse(SomeObject argument1) {
        // ...
    }

}

This way you cannot override them.

If yours is not located in kind of an utility class, then I'd question the value of the public modifier. Shouldn't it be private? Else just move it out to some utility class. Do not clutter "normal" classes with public static methods. This way you also don't need to mark them final.

Another case is a kind of abstract factory class, which returns concrete implementations of self through a public static method. In such case it would perfectly make sense to mark the method final, you don't want the concrete implementations be able to override the method.

BalusC
+2  A: 

Static methods are one of Java's most confusing features. Best practices are there to fix this, and making all static methods final is one of these best practices!

The problem with static methods is that

  • they are not class methods, but global functions prefixed with a classname
  • it is strange that they are "inherited" to subclasses
  • it is surprising that they cannot be overridden but hidden
  • it is totally broken that they can be called with an instance as receiver

therefore you should

  • always call them with their class as receiver
  • always call them with the declaring class only as receiver
  • always make them (or the declaring class) final

and you should

  • never call them with an instance as receiver
  • never call them with a subclass of their declaring class as receiver
  • never redefine them in subclasses

 

NB: the second version of you program should fails a compilation error. I presume your IDE is hiding this fact from you!

Adrian
Very nice answer! By the way, the second version does indeed fail compilation in my IDE. Edited the question to clarify this.
brass-kazoo