tags:

views:

1710

answers:

7

I am working on some legacy code and have come across something that I'm not sure of. We have a class y that is declared inside of another class x. Class y is only ever used inside of class x but my question is why wouldn't you create a separate class file and put class y in there instead of declaring it inside of class x? Isn't this violating OOP's or is it just a matter of style since it is only ever used inside of this class. I'm refactoring some of this code and my first reaction would be to separate class y out into it's own file.

namespace Library
{
   public class x
   {
      // methods, properties, local members of class x

      class y
      {
         // methods, properties, local members of class y
      }
   }
}
+23  A: 

You create an inner class because it is only ever used within the scope of class x and it logically fits in the factoring/architecture of class x.

Class y might also be privy to implementation details of class x that are not meant to be known to the public.

plinth
+15  A: 

This has permissions implications. A top-level "class y" would be "internal" - however, here "y" is private to "x". This approach is helpful for implementation details (for example cache rows etc). Likewise, y has access to all private state of x.

There are also implications with generics; x<T>.y is generic "of T", inherited from the outer class. You can see this here, where Bar has full use of T - and note that any static fields of Bar are scoped per-T.

class Foo<T> {
    void Test(T value) {
        Bar bar = new Bar();
        bar.Value = value;
    }
    class Bar {
        public T Value { get; set; }
    }
}

Often people incorrectly think they need to define Bar as Bar<T> - this is now (effectively) doubly generic - i.e. Foo<TOld, T> - where TOld is the (now unavailable) T from Foo<T>. So don't do that! Or if you want it to be doubly-generic, pick different names. Fortunately, the compiler warns you about this...

Marc Gravell
+1 It might be a good idea to explain how "T" from the outer class will be part of the closed constructed type of the inner class. It is an important point that many folks are unaware of.
Andrew Hare
+2  A: 

I think it's ok, as long as the contained class is only used as utility. I use this sort of construct for example to define complex return types for private methods.

phatoni
+2  A: 

I just went through code that I am updating (and I originally wrote) and removed all nested classes. Unfortunately, I originally used the nested class outside of the class it was defined in. Moving nested classes out made a huge difference to me because I originally had bad design.

If Y is only used in X and will never be used outside of X, I'd say keep it there

phsr
+5  A: 

This code is fine for the exact reason that you have given - "class y is only ever used inside of class x". Those are nested types and one of the guidelines for using them is that nested types should be tightly coupled to their declaring type and must not be useful as a general purpose type. That way the nested class is inacessible to other classes, but still allows you to follow object oriented principles.

Pawel Krakowiak
+1  A: 

Let me give you an example of the use of nested classes that might clarify when this kind of architecture is appropriate. I recently needed to generate an HTML table by pulling selected columns from a data table and "pivoting" them so that rows become columns and vice versa. In my case, there were two essential operations: pivoting the data and generating some rather complex output (I was not just showing the data: each data column/table row was subject to operations for extracting title, generating image tags, setting up links, etc. thus using a SQL Pivot wasn't really right either).

After an initial attempt to create one class to do the whole thing, I recognized that much of the data/methods fell into three distinct partitions: header processing, row processing, and pivoting. Thus, I decided that a better approach would be to encapsulate the logic for "header" and "row" into separate, nested classes. This allowed me to separate the data held by each row and program the pivot operations very cleanly (calling a separate row object for each column in your data table). At the end of the pivot operations, I generated output by calling the header object and then each row object in turn to generate its output back to the main class.

Separate classes weren't appropriate because A) the nested classes did need some data from the master class and B) the processing was very specific and not useful elsewhere. Just programming one big class was simply messier due to confusion surrounding terms such as "column" and "row" which differed depending on whether you were talking about data or HTML output. Also, this was unusual work in that I was generating HTML in my business class so I wanted to pull apart the pure business logic from the UI generation. In the end, nested classes provided the perfect balance, then, of encapsulation and data sharing.

Mark Brittingham
+1  A: 

You could still refactor your class y into another file, but use a parial class. The benefit of this is that you still have one class per file and don't have the refactoring hassles of moving the declaration outside of class x.

e.g. you could have a code file: x.y.cs which would look something like

partial class X
{
    class Y
    {
        //implementation goes here
    }
}
mdresser