Using inner classes to reflect relationships of real things

Question

This is more of a conceptual question. I know how to do what I'm trying to do. I'm wondering if it's the right thing to do though.

I'm trying to represent something that involves a bit of nesting in real life. It is a document that specifies activities to be performed with a set of items. A document may cover multiple items and each item may have multiple activities.

So the hierarchy will be Document -> Item -> Activity.

My current thinking is to represent this will one top level class, Document, which contains an inner class, ItemProgram, which itself contains an inner class, Activity. Yes, that's two levels of nesting.

public class Document {

   // Properties of the document itself

   private Map<Item, ItemProgram> itemPrograms; // Map of item programs

   public class ItemProgram {

      // Properties of the item program itself

      private List<Activity> activities; // List of activities

      public class Activity {
         // Properties of the activity
      }
   }
}

The inner classes must be public, but I'm making constructors private so that they are only created by add methods in their outer classes. As you can see, I store the instances using a type of Collection.

Is this the right way to go about this? Double nesting?

Is it appropriate to use collections in the outer class to store references to all instances of the inner class?

Answer 1

I can see why you are tempted to use this construct, but it's not something I would choose (of course there is no absolute right/wrong answer). Here are a few things to consider:

The inner class instances will have access to the internal state of their enclosing objects, which is probably not necessary for your example, and will make the classes very tightly coupled.

Double nesting in this way is 'unusual' and might confuse other people who read your code.

In the future, if the design of your system changes and it becomes possible for an Activity to be created independently of an Item (for example) then you will have to perform some major surgery in your code.

Instances of the inner classes will be created with an implicit reference to their parent object, do you actually need this extra reference (the parents already know about their children via the private collections, does it have to be a two-way relationship which adds a certain amount of complexity)?

Have you thought about how this design might affect the testability of your code? What if you want to unit-test the Activity class - could you do this in such a way that a test failure would definitely indicate a problem in Activity, and not in one of the parent classes?

Answer 2

This particular approach is fine while there is one type of each, keep in mind (and document in comments) that if you start to need multiple types or derivations of Document, Activity or ItemProgram you will most likely need to split this class apart to make a more scalable pattern.

If Document contains its own logic, plus logic of creation of Activity instances and logic for the creation of ItemProgram instances it is against the guidance of SRP (Single Responsibility Principle). This principle helps you prevent issues in scale out and prevent class explosions, where you start getting classes representing combinations rather than using compositional patterns.

I would also say nested classes are ok if the clients of those classes are only the parent (enclosing) classes and I would separate it out if you are using them elsewhere.

EDIT Having said all of that, there is no benefit to nesting the classes over having them as separate classes, at least no benefit that would positively effect the system you are developing:

• Hidden complexity, what other developer will know that the file contains three classes and not what they expect, one.
• Access to privates, parents can access nested class variables as well as the other way around, either way it isn't expected and will be hard to maintain.