Are there any drawbacks to using a nested class instead of declaring a new one?

Question

I'm doing code review on a change my co-worker made to our Java application, and I've found something I'm not very familiar with - a nested class.

From reviewing the code, it seems like the nested class is being used as if it were a normal class - and asking my co-worker about it, the reason for putting it in as a nested class is because of a source control issue preventing her from creating a new class on the day she coded it.

Now this is bothering me - because while there's no reason to introduce this element into our code (very few classes use nested classes in our application), there's also no drawbacks to it that I can think of either. The nested class is, in a very loose way, related to the class it came from, and re-writing the code so that the nested class is an independent one would take some time.

Is there any good reason to have my co-worker redo the code so that this nested class is independent? Or would I just be asking them to waste their time on something that does not matter?

Note that there does not appear to be any functional affect on implementing the class this way - so any argument would have to be from best practice or bad structure, rather than trying to prove that it doesn't work (because it does work - I'm just not sure it's appropriate).

Answer 1

If the nested class is private then it is part of the implementation details of that class. There are various valid reasons why such a class might exist: data encapsulation, proving a private implementation of an interface to name two.

If the class is accessible outside of its containing class, then the single responsibility principle likely comes into play. Is that inner class genuinely a responsibility of the outer one? Since you say it’s only loosely related, then the answer is likely, no.

Nesting classes tightly couples them. It makes the outer class more complex as it now contains two class’s worth of functionality. And that outer class now has two responsibilities. And all this exists because of a problem with a check-in.

So I’d definitely recommend restructuring the code in this case too move that inner class out into its own file where it belongs.

Answer 2

I'm obligated to post this cartoon about the measure of code quality.

If the only reason for creating the nested class instead of a free-standing class was a clerical restriction [source control is an automated clerk], then the nested class should be refactored into a free-standing class. If there are no other reasons, this should be an easy refactoring.

Answer 3

Given that it was only created due to a problem with source control rarther than any design considerations then yes. You should probably refactor it.

However, if the code is functional and you don't have a "no nested classes" coding style rule then I would not recommend you fail the code review because of it.