For some time, I've been using public nested classes (often records) for POCOs (Plain Old C# Object), mostly with MediatR and services. I have found this approach very useful. Everything is contained within the MediatR model (or service), there is no way to access subclasses without explicitly understanding what you are doing.
IDEs are also working great with this approach. Let's say you have MediatR queries: GetEntity, GetEntityList, SearchEntity, etc. That is at least 3 different entity models. With nested classes, it is very easy to name those entities GetEntity.Entity, GetEntityList.Entity, SearchEntity.Entity. But, how are you going to name them without nested classes? Just "entity" and use namespaces? Try to do that in Visual Studio, ReSharper, Rider, etc. IDEs will passionately work against you.
Search Everywhere (Rider) and Navigate To (Visual Studio) work better with nested classes, than with namespaces.
Also, why do we have to pollute excellent Jump To features of modern IDEs with search results that we will never need? We will never edit POCO in a vacuum! POCO by definition 'needs' a larger context and group of classes (although, by this definition, we can argue that every class is meaningless in isolation).
Are public nested classes bad practice?
At the time of inception, in C# (similar to Java) nested classes were supposed to be used mainly as private and only when we need to hide the inner workings of the class.
I have started doubting my opinion mainly because of Eric Lippert's comment on public nested classes:
The contents of a class should be the implementation details of that class. Are the nested classes implementation details of the outer class, or are you merely using the outer class as a convenient name scoping and discovery mechanism?
If the former, then you shouldn't be making the private implementation details publically available. Make them private if they are implementation details of the class.
If the latter, then you should be using namespaces, not outer classes, as your scoping and discovery mechanism.
Either way, public nested classes are a bad code smell. I'd want to have a very good reason to expose a nested class.
Eric Lippert - StackOverflow - c# Public Nested Classes or Better Option?
However, John Skeet gave a more open answer to the same question:
I don't have too much problem with public nested classes (I'm not a fan of dogmatic rules, in general) but have you considered putting all of these types in their own namespace instead? That's the more common way of grouping classes together.
EDIT: Just to clarify, I would very rarely use public nested classes, and I probably wouldn't use them here, but I wouldn't completely balk at them either. There are plenty of examples of public nested types in the framework (e.g.
List<T>.Enumerator) - no doubt in each case the designers considered the "smell" of using a nested class, and considered it to be less of a smell than promoting the type to be a top-level one, or creating a new namespace for the types involved.
Microsoft's documentation is similar:
Public nested types should be used rarely
Nested Type Usage Guidelines - Microsoft Documentation (2006)
DO use nested types when the relationship between the nested type and its outer type is such that member-accessibility semantics are desirable.
DO NOT use public nested types as a logical grouping construct; use namespaces for this.
AVOID publicly exposed nested types. The only exception to this is if variables of the nested type need to be declared only in rare scenarios such as subclassing or other advanced customization scenarios.
DO NOT use nested types if the type is likely to be referenced outside of the containing type.
Nested Types - Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries, 2nd Edition (2008)
However, Kathleen Dollard offers arguments for nested classes in her 2008 blog post In Praise of Nested Classes.
Also in the official Java documentation for Nested Classes, there are a few arguments as well:
Compelling reasons for using nested classes include the following:
It is a way of logically grouping classes that are only used in one place: If a class is useful to only one other class, then it is logical to embed it in that class and keep the two together. Nesting such "helper classes" makes their package more streamlined.
...
It can lead to more readable and maintainable code: Nesting small classes within top-level classes places the code closer to where it is used.
Conclusions
- Am I doing it wrong?
- Am I breaking some 'holly rule'?
- Do we need to follow Microsoft's 15-plus-year-old rules to the letter?
I don't know.
If you use MediatR with API - you will (almost) never initialize classes manually, controllers will deserialize them directly into objects. For MediatR results, you usually have to initialize objects manually, but I am still finding containing MediatR results and input models inside the request classes very useful. I want my MediatR-s to have a single purpose - only to be used with that query/command and handler pairs.
I will probably continue to use public nested classes with POCOs for logical grouping. With modern IDEs, I still find grouping with namespaces nowhere as elegant as nested classes. I don't want to sound disrespectful, and I am open to changing my practices, but I would like to hear something other than "you are not supposed to do that."