Large Inner classes and private variables

Answer 1

Yeap. Probably you need to re-refactor those helpers and not move them all as they are. Some things belong to the service some other to the helper. Probable new classes should be used to encapsulate the data.

One posibility you can use is to AOP to provide fine-grained access, and include in the point cut that the method should be only invoked from the "friend" class. Still your method would be exposed :(

I guess there is no easy solution for this.

Answer 2

The line between encapsulation and separation can be difficult to walk. However, I think the main issue here is that you need some kind of solid interaction model to use as a basis of separating your classes.

I think it's reasonable to have external helper utility classes which are used in many places, as long as they don't side effect I don't see an issue. It's also reasonable to have static helper classes, as long as they are well organized, which contain often used methods such as checkAssetIsValid(). This is assuming that checkAssetIsValid does not need to access any external state other than the object you are passing it.

The most important thing for separation is not to have objects which share permanent references in many of these classes. I like to look to functional programming for guidance. Each class should not be reaching into the guts of other classes and changing state. Instead each class which does work should produce and consume container objects.

Visualization can be very helpful too. I noticed a thread on the topic of Java Visualization tools here. Ideally, your class interaction diagram should look more like a tree than a graph.

Also, I just want to note that refactoring a large class into smaller classes can be extremely difficult. It's best to build a suite of unit tests for the public interface at the very least so that it will become immediately obvious if you break something. I know tests have saved me countless hours in the past.

Hopefully some of this will be helpful. I'm kind of just rambling on here.

Answer 3

Don't forget to consider why you're trying to break up your large class. Is it for software engineering purposes? E.g. it's a programming hotspot and you have such a large file it causes complicated merges on your developer team?

Is it just a general desire to avoid large classes? In which case it may be that your time would be better spent improving the code you do have.

Is the code becoming difficult to manage, e.g. debugging and ensuring avoidance of unintended side effects is becoming more difficult.

Rick's comment about using unit tests to ensure continued consistent behavior is very valuable and a good idea. It may be that the current design simply precludes refactoring and you're better off re-implementing the same behavior starting from the interface of the original. Be ready for plenty of regresssion testing!

Answer 4

I'm not a fan of excessive use of inner classes. I think that they don't really offer any advantage (when used in the extreme) that putting the code in a normal class wouldn't, and they just serve to make the class file unnecessarily large and hard to follow.

What's the harm if you have to increase the visibility of a few methods? Is it going to completely break your abstraction or interface? I think too often programmers tend to make everything private by default, where there isn't really much of a harm in letting some other classes call your methods - if your design is truly OO-based, that is.

If all of your "inner helper classes" need access to some of the same methods, consider putting them in a base class so they can be shared via inheritance.

Answer 5

Extract all of the code you can from the helper classes to their enclosing service. If those operations need access to the enclosing object's private state, they should be methods of that object, not some other class.

Often, inner classes are (or should be) adapters, so that operations of the adaptee (enclosing class) can be invoked through some other API. For example, Thread requires a Runnable target, and you want to target it at your service, so you create a Runnable implementation as an inner class.

Of course, this won't make the overall class any smaller. But it will put you in a better position to factor out the several responsibilities of a overly complex class.

Answer 6

On bytecode level inner classes are just plain Java classes. Since the Java bytecode verifier does not allow access to private members, it generates synthetic accessor methods for each private field which you use. Also, in order to link the inner class with its enclosing instance, the compiler adds synthetic pointer to the outer 'this'.

Considering this, the inner classes are just a layer of syntax sugar. They are convenient and you have listed some good points, so I'd list some negative aspects which you might want to consider:

• Your inner class has a hidden dependency to the whole parent class, which obfuscates its inbound interface. If you extract it as package-private class you have a chance to improve your design and make it more maintainable. Initially it's more verbose, but often you'd find that:
  • Instead of exposing 10 accessors you actually want to share a single value object. Often you would find that you don't really need a reference to the whole outer class. This also works well with IoC.
  • Instead of providing methods for explicit locking, it's more maintainable to encapsulate the operation with its context in a separate class (or move it to one of the two classes - outer or formerly-inner).
  • Convenience methods belong in package private utility classes. You can use the Java5 static import to make them appear as local.
• Your outer class can bypass any protection levels and access private members of your inner class directly. This is not bad thing per se, but it takes away one of the language means of expressing your design.
• Since your inner class is embedded in exactly one outer class, the only way to reuse it is to subclass the outer class. An alternative would be to pass explicit reference to a package-private interface that the outer class implements. This would allow you to mock the outer and better test test the inner class.
• Though recent debuggers are quite good, I have experienced problems with debugging inner classes before (conditional breakpoint scope confusion, not stopping at breakpoints, etc.)
• Private classes bloat your bytecode. See my first paragraph - often there is an API that you could use and reduce the number of synthetic cruft.

P.S. I'm talking about non-trivial inner classes (especially ones that do not implement any interfaces). Three line listener implementations are good.