views:

168

answers:

3

I have inherited code which contains static nested classes as:

public class Foo {

// Foo fields and functions
// ...
    private static class SGroup {
        private static Map<Integer, SGroup> idMap = new HashMap<Integer, SGroup>();

        public SGroup(int id, String type) {
// ...
        }
    }
}

From reading SO (e.g. http://stackoverflow.com/questions/70324/java-inner-class-and-static-nested-class) I believe that this is equivalent to two separate classes in two separate files:

 public class Foo {

    // Foo fields and functions
    // ...
}

and

public class SGroup {
    static Map<Integer, SGroup> idMap = new HashMap<Integer, SGroup>();

    public SGroup(int id, String type) {
// ...
    }
}

If this is correct is there any advantage to maintaining the static nested class structure or should I refactor?

+8  A: 

It depends on what the class is used for. If it's coupled to the outer class, for example, just like Map.Entry, just leave it in. However, if it makes sense to use the class without its enclosing type, you may as well promote it to a top level class.

Jorn
+5  A: 

Jorn statement is correct and it's usually manifests itself as the following rule of thumb:

Nested classes should be made private, Meaning that the hold auxiliary logic for the hosting class and nothing more. If you cant make them private- thet probably should not be nested.

The exception is when you define a nested class to allow easy access to the state of the hosting class, in that case you should consider simply merging both classes to increase cohesion.

Vitaliy
A: 

I like static inner classes as they provide loose coupling from the enclosing class (no access to private members of the enclosing class) static inner classes are also easy to promote to top level (because of the loose coupling attribute).

There is a simple rule of the thumb when to promote them:
If another class (other than the enclosing) needs to reference \ use the inner class.

LiorH
Static inner classes may access private members of their enclosing class, so lose coupling is not ensured. (Granted, they need an instance of their enclosing class to access the non-static ones).So you would factor out java.util.Map.Entry into a top-level class? I beg to differ.
meriton
how does Map.Entry falls into the rule I suggested? it's neither static nor a class. Though I agree that passing a reference of the enclosing class to the inner class bypass the whole idea of loose coupling.
LiorH
`Map.Entry` being an interface is implicitly static.
Tom Hawtin - tackline