1

When I began implementing a class in my system (let's call it A), I realized I needed some kind of object that does something. So I needed a new class (let's call it B), but I made it a private static class of A.

Now, B is still only ever used by A, but A grew much bigger. Including the code for B, it's 320 LoC long.

The question is, should I move B outside of A, into a separate package-private class? It's still only ever used by A, but since A is getting big I think I should split it into smaller parts.

Stephen
  • 8,800
  • 3
  • 30
  • 43
Aviv Cohn
  • 21,190
  • 31
  • 118
  • 178
  • To me the determining criteria is something you don't have listed here: does B make sense by itself? That is to say, could some future class C reasonably reuse B? Or is truly some helper structure/utility? That's the only objective thing I can think of, beyond that I think it will revert to preference. In my mind, 320 LoC is still relatively tameable so that wouldn't be a deciding factor for me. – J Trana Sep 24 '14 at 00:50
  • one may argue that conceptually, this has been addressed in [How would you know if you've written readable and easily maintainable code?](http://programmers.stackexchange.com/a/141010/31260) If your peers keep complaining about your way of doing things, be it one way or another, you better change to make them feel better – gnat Sep 24 '14 at 07:18

2 Answers2

1

The size of class A doesn't really matter, provided it follows the following criteria:

  1. It's only responsible for one thing
  2. It groups logically related functionality

As long as that criteria is fulfilled, you do not need to split up the functionality into separate classes. The difficulty is in determining whether the class is actually responsible for more than one thing.

I tend to avoid private classes as they clutter up code. The only reason I would create a private class usually is for a data container for temporary data which is only used by that class. In pretty much every other circumstance I would create classes separately from each other.

Stephen
  • 8,800
  • 3
  • 30
  • 43
  • Why would data in a static nested class be a problem? – COME FROM Nov 24 '14 at 10:47
  • If you have two instances of the parent class they could alter the state of the static private class and cause bugs. Anything which has state should be instanced. – Stephen Nov 25 '14 at 01:30
  • I don't quite understand your point. What do you exactly mean by "static private class"? [A static nested class?](https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.5.1) If the nested class has static fields, they should be immutable just like all visible static fields of any class should be. The static nested class should be instanced and used just like an inner class (non-static nested class). It just doesn't need an implicit reference to an instance of the enclosing type. – COME FROM Nov 25 '14 at 12:45
  • I'm not sure I understand your point. All I was saying is that if you are going to use a static class (regardless of whether it is nested) be aware that it should never have a state. Anything which has state should be in an instantiated (i.e. non-static) class. – Stephen Nov 26 '14 at 11:29
  • My point is that a static class can and usually it should be instantiated. Just call the constructor: `new NestedStatic()`. If you call it twice, you get two separate objects with separate states. I think you're confusing _static classes_ with _classes that have only static members_. – COME FROM Nov 27 '14 at 13:13
  • Note that all package level classes are just like static nested classes in the sense that they don't have an implicit reference to an instance other than themselves like an inner class does. Inner class (non static nested class) is the exceptional case really. It can be hard to refactor an inner class into a static nested or into a package level class. References to the enclosing instance need to be solved in the process. A static nested class can't have such implicit dependency and thus it's generally safer (IMHO). – COME FROM Nov 27 '14 at 13:26
  • It seems that [C# does not have inner classes](http://msdn.microsoft.com/en-us/library/ms173120.aspx) like Java does. Perhaps that is the root of this confusion? In Java terms all C# nested classes are "static"... :) – COME FROM Nov 27 '14 at 13:50
  • Maybe I am confusing the java concept of a static class with the C# one. I was under the impression that they were the same (it has been a decade since I wrote java in anger). – Stephen Nov 27 '14 at 23:23
  • Since this is a Java question, shouldn't you edit the answer? Your answer could mislead some developers as your definition of "static class" does not correspond to a static class in Java. – COME FROM Nov 28 '14 at 07:35
  • I read up on the different way that Java and C# treat nested classes. I must say that I prefer the way that C# handles them, though that could just be because I've used it more. I have edited the answer to remove the incorrect information. – Stephen Nov 28 '14 at 10:25
  • Thanks for the edit. I am not a huge fan of Java's inner classes either and I guess excluding them is a well thought design choice in C#. Now that your answer doesn't (accidentally) advocate using them, I give it +1 – COME FROM Nov 28 '14 at 10:58
0

I try to avoid inner classes in general unless they are simple lambdas, listeners with one or two lines of code, etc.

Once an inner class has a life of its own and performs more complex functions, I refactor it into its own top-level class. I will use either a standard interface or create my own for the class that used to enclose it (A). I then construct the B class and pass in a reference to the interface that A now implements and is used by B.

While A and B are tightly coupled, they probably do not need to be. Severing that coupling and using interfaces and references rather than an implicit outer-class this pointer/reference now means B is one step closer to being reusable. There is now an abstraction between them.

Often the reason for introducing another class is that the enclosing class needs to do additional work that it cannot do itself. That additional work is likely (in my experience) to violate the Single Responsibility Principle. Moving it into an inner class is one step toward fixing that, but is (in my opinion) a suboptimal fix. Refactoring as I mentioned above will fix the tight coupling and "multiple responsibility" concerns.

One final reason to split into multiple top-level classes is it makes the source files shorter. This has nothing to do with OOP principles and is purely aesthetic.