21

Suppose I have a class 'Application'. In order to be initialised it takes certain settings in the constructor. Let's also assume that the number of settings is so many that it's compelling to place them in a class of their own.

Compare the following two implementations of this scenario.

Implementation 1:

class Application 
{
   Application(ApplicationSettings settings) 
   {
       //Do initialisation here
   }
}

class ApplicationSettings 
{
   //Settings related methods and properties here
}

Implementation 2:

class Application 
{
   Application(Application.Settings settings) 
   {
       //Do initialisation here
   }

   class Settings 
   {
      //Settings related methods and properties here
   }
}

To me, the second approach is very much preferable. It is more readable because it strongly emphasises the relation between the two classes. When I write code to instantiate Application class anywhere, the second approach is going to look prettier.

Now just imagine the Settings class itself in turn had some similarly "related" class and that class in turn did so too. Go only three such levels and the class naming gets out out of hand in the 'non-nested' case. If you nest, however, things still stay elegant.

Despite the above, I've read people saying on StackOverflow that nested classes are justified only if they're not visible to the outside world; that is if they are used only for the internal implementation of the containing class. The commonly cited objection is bloating the size of containing class's source file, but partial classes is the perfect solution for that problem.

My question is, why are we wary of the "publicly exposed" use of nested classes? Are there any other arguments against such use?

Jon Seigel
  • 12,251
  • 8
  • 58
  • 92
G S
  • 35,511
  • 22
  • 84
  • 118

6 Answers6

28

I think it's fine. This is basically the builder pattern, and using nested classes works pretty well. It also lets the builder access private members of the outer class, which can be very useful. For instance, you can have a Build method on the builder which calls a private constructor on the outer class which takes an instance of the builder:

public class Outer
{
    private Outer(Builder builder)
    {
        // Copy stuff
    }

    public class Builder
    {
        public Outer Build()
        {
            return new Outer(this);
        }
    }
}

That ensures that the only way of building an instance of the outer class is via the builder.

I use a pattern very much like this in my C# port of Protocol Buffers.

Hamlet Hakobyan
  • 32,965
  • 6
  • 52
  • 68
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • I had to write some builder code recently and remembered this post. It turned out to be a very useful approach, so thanks. – Mark Simpson Jun 12 '10 at 13:33
  • *"It also lets the builder access private members of the outer class"* Only with an explicit reference, correct (I'm pretty sure it's not implicit like it is in Java's inner-classes)?. – samus Jun 19 '17 at 17:44
  • @SamusArin: Yes, there's no implicit reference to an instance of the containing class. But you also have access to static members. Basically I was *only* talking about accessibility. – Jon Skeet Jun 19 '17 at 17:45
6

You can use namespaces to relate things that are... related.

For example:

namespace Diner
{
    public class Sandwich
    {
        public Sandwich(Filling filling) { }
    }

    public class Filling { }
}

The advantage of this over using classes as if they were namespaces is that you can optionally use using on the calling side to abbreviate things:

using Diner;

...

var sandwich = new Sandwich(new Filling());

If you use the Sandwich class as if it were a namespace for Filling, you have to use the full name Sandwich.Filling to refer to Filling.

And how are you going to sleep at night knowing that?

Daniel Earwicker
  • 114,894
  • 38
  • 205
  • 284
  • 2
    I agree with the preference for namespaces, but sometimes they don't quite work. In the example in question, for instance, semantically 'Application' subsumes 'Settings', but to put them both in a common namespace wouldn't convey that meaning. – G S Dec 04 '08 at 04:53
  • **`using Filling = Diner.Filling;`** should resolve "references" to (just) **`Filling`** in the case of "classes as a namespace". – samus Jun 19 '17 at 17:56
1

You might want to check out what Microsoft has to say on the topic. Basically it's a question of style I'd say.

Vilx-
  • 104,512
  • 87
  • 279
  • 422
  • .NET doesn't impose the "one (public) class per file" rule that Java does. They seem to have distinct paradigms to program structure. WRT this one rule alone, .NET has more flexibility (an expressive super-set if you will). Great article btw. – samus Jun 19 '17 at 19:38
0

I primarily use nested classes for fine-tuning access to the nested and/or the container class.

One thing to remember is that a nested class definition is basically a class member, and will have access to all the container's private variables.

You can also use this to control usage of a specific class.

Example:

public abstract class Outer
{
  protected class Inner
  {
  }
}

Now, in this case, the user (of your class) can only access the Inner class, if he implements Outer.

leppie
  • 115,091
  • 17
  • 196
  • 297
  • 1
    Iff may not be a typo, but it *is* somewhat redundant since you already have the word "only" in the sentence. – phoog Dec 21 '15 at 15:29
0

Another practical example that I have for a valid use of public nested classes is in MVC pattern when I use a viewmodel with an IEnumerable property. for example:

public class OrderViewModel
{
public int OrderId{ get; set; }
public IEnumerable<Product> Products{ get; set; }

public class Product {
public string ProductName{ get; set; }
public decimal ProductPrice{ get; set; }
}

}

I use it because I don't want Product class to be re-used outside because it is customized only for that specific viewmodel which contains it. But I can't make it private because the Products property is public.

BornToCode
  • 9,495
  • 9
  • 66
  • 83
-1

I don't know if this is considered bad design or not, but I've got some search classes I make where a user calls the Run() method, passing in an object that holds search criteria. It then returns a collection of search result objects.

These SearchCriteria and SearchResult classes have no utility outside of using them with the Search class. So I nest them under the Search class to show they go together.

I have to make the nested classes public so the client of the Search class can make the SearchCriteria to pass into the Search class and so they can get the results of the Search.

public class PersonSearch
{
    public PersonSearchCriteria
    {
        string FirstName {get; set;}
        string LastName {get; set;}
    }

    public PersonSearchResult
    {
        string FirstName {get;}
        string MiddleName {get;}
        string LastName {get;}
        string Quest {get;}
        string FavoriteColor {get;}
    }

    public static List<PersonSearchResult> Run(PersonSearchCriteria criteria)
    {
        // create a query using the given criteria

        // run the query

        // return the results 
    }
}


public class PersonSearchTester
{
    public void Test()
    {
        PersonSearch.PersonSearchCriteria criteria = new PersonSearch.PersonSearchCriteria();
        criteria.FirstName = "George";
        criteria.LastName  = "Washington";

        List<PersonSearch.PersonSearchResults> results = 
            PersonSearch.Run(criteria);
    }
}
cbast
  • 21
  • 1
  • 4