Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Large Inner classes and private variables

One thing I've run into a few times is a service class (like a JBoss service) that has gotten overly large due to helper inner classes. I've yet to find a good way to break the class out. These helpers are usually threads. Here's an example:

/** Asset service keeps track of the metadata about assets that live on other
 * systems. Complications include the fact the assets have a lifecycle and their
 * physical representation lives on other systems that have to be polled to find
 * out if the Asset is still there. */
public class AssetService
{
  //...various private variables
  //...various methods

  public AssetService()
  {
    Job pollerJob = jobService.schedule( new AssetPoller() );
    Job lifeCycleJob = jobService.schedule( AssetLifecycleMonitor() );
  }

  class AssetPoller
  {
    public void run()
    { 
      // contact remote systems and update this service's private variables that
      // track the assets.
    }
  }

  class AssetLifecycleMonitor
  {
    public void run()
    {
      // look for assets that have meet criteria for a lifecycle shift
      // and update this service's private variables as relevant.
    }
  }
}

So, what can happen if I have a couple helpers and they're at all complex, is the overall class file can get really large. I like the inner classes in that it makes clear the classes are wholly owned by the service and exist only to help that service. I've tried breaking the classes out and passing the parent service as a reference, which works mostly, but things I don't like are:

  • I end up exposing package level accessors so the broken out classes can get to the variables, whereas before I didn't expose the setters at all since the inner classes had direct access.
  • Plus, things get a bit more wordy as I'm constantly calling accessors rather than the underlying variables. A minor nit, granted.
  • Convenience methods (e.g. checkAssetIsValid() or some such) need package level exposure now so the helper classes can call them, where as before as inner classes they could be private.
  • Even worse, I need to pass the service implementation class to the helper classes constructors since I don't want to expose these helpers methods in the interface the service implements because that forces them to be public. This can create some unit test/mocking issues.
  • Worse yet, any synchronization I wanted to do gets leaked out through some external convenience method (e.g. lockDownAssets() during a poller update). Before, the internal classes had access to private Locks.

    So, in short, breaking the classes out loses some of the encapsulation I like. But leaving them in can lead to some large java files. I've yet to find a good way to deal with this. C++ had the concept of "friends" which I've rarely missed, but would actually help in this case.

    Thoughts?

  • like image 581
    Chris Kessel Avatar asked Oct 20 '08 19:10

    Chris Kessel


    1 Answers

    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 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.

    like image 125
    2 revs, 2 users 96% Avatar answered Oct 28 '22 22:10

    2 revs, 2 users 96%