tags:

views:

1649

answers:

9

What are the pros and cons of using nested public C++ classes and enumerations? For example, suppose you have a class called printer, and this class also stores information on output trays, you could have:

class printer
{
public:
    std::string name_;

    enum TYPE
    {
        TYPE_LOCAL,
        TYPE_NETWORK,
    };

    class output_tray
    {
        ...
    };
    ...
};

printer prn;
printer::TYPE type;
printer::output_tray tray;

Alternatively:

class printer
{
public:
    std::string name_;
    ...
};

enum PRINTER_TYPE
{
    PRINTER_TYPE_LOCAL,
    PRINTER_TYPE_NETWORK,
};

class output_tray
{
    ...
};

printer prn;
PRINTER_TYPE type;
output_tray tray;

I can see the benefits of nesting private enums/classes, but when it comes to public ones, the office is split - it seems to be more of a style choice.

So, which do you prefer and why?

+6  A: 

One con that can become a big deal for large projects is that it is impossible to make a forward declaration for nested classes or enums.

Adam Rosenfield
Note that using a namespace to nest a class enable the user to forward-declare this class: i.e.: <code>namespace Foo { class Bar ; }</code>. Anyway, +1 for the remark.
paercebal
+18  A: 

Nested classes

There are several side effects to classes nested inside classes that I usually consider flaws (if not pure antipatterns).

Let's imagine the following code :

class A
{
   public :
      class B { /* etc. */ } ;

   // etc.
} ;

Or even:

class A
{
   public :
      class B ;

   // etc.
} ;

class A::B
{
   public :

   // etc.
} ;

So:

  • Privilegied Access: A::B has privilegied access to all members of A (methods, variables, symbols, etc.), which weakens encapsulation
  • A's scope is candidate for symbol lookup: code from inside B will see all symbols from A as possible candidates for a symbol lookup, which can confuse the code
  • forward-declaration: There is no way to forward-declare A::B without giving a full declaration of A
  • Extensibility: It is impossible to add another class A::C unless you are owner of A
  • Code verbosity: putting classes into classes only makes headers larger. You can still separate this into multiple declarations, but there's no way to use namespace-like aliases, imports or usings.

As a conclusion, unless exceptions (e.g. the nested class is an intimate part of the nesting class... And even then...), I see no point in nested classes in normal code, as the flaws outweights by magnitudes the perceived advantages.

Furthermore, it smells as a clumsy attempt to simulate namespacing without using C++ namespaces.

On the pro-side, you isolate this code, and if private, make it unusable but from the "outside" class...

Nested enums

Pros: Everything.

Con: Nothing.

The fact is enum items will pollute the global scope:

// collision
enum Value { empty = 7, undefined, defined } ;
enum Glass { empty = 42, half, full } ;

// empty is from Value or Glass?

Ony by putting each enum in a different namespace/class will enable you to avoid this collision:

namespace Value { enum type { empty = 7, undefined, defined } ; }
namespace Glass { enum type { empty = 42, half, full } ; }

// Value::type e = Value::empty ;
// Glass::type f = Glass::empty ;

Note that C++0x defined the class enum:

enum class Value { empty, undefined, defined } ;
enum class Glass { empty, half, full } ;

// Value e = Value::empty ;
// Glass f = Glass::empty ;

exactly for this kind of problems.

paercebal
Good example of a nested private class is the links in the list inside a std::list. There is no need for anybody to know about them use them or interact with them (or do they even really exist ;-)
Martin York
I do use a nested private class every now and then for the pImpl-pattern.
xtofl
The nested private class in the PImpl pattern means the user will see the implementation class, even if it is not accessible. Having a simple forward declaration will only declare the name, and nothing else. You'll be able to define fully the implementation elsewhere (hidden from the user)... ^_^ ..
paercebal
The forward declaration problem is more than just an inconvenience. You can get yourself stuck in a dependency loop that requires un-nesting of the problem area. The un-nesting could then create design inconsistency, or you would have to change your whole project to un-nested : / http://stackoverflow.com/questions/951234/forward-declaration-of-nested-types-classes-in-c
Catskul
@Catskul: You're right about the forward-declaration. +1
paercebal
I completed the "nested classes" section after further investigation (prompted by a technical discussion with a friend). The results are the same (unless exception, nested classes are NOT Ok), but the reasons are better detailed.
paercebal
+2  A: 

If you're never going to be using the dependent class for anything but working with the independent class's implementations, nested classes are fine, in my opinion.

It's when you want to be using the "internal" class as an object in its own right that things can start getting a little manky and you have to start writing extractor/inserter routines. Not a pretty situation.

Paul Nathan
+2  A: 

It seems like you should be using namespaces instead of classes to group like things that are related to each other in this way. One con that I could see in doing nested classes is you end up with a really large source file that could be hard to grok when you are searching for a section.

carson
A: 

For me a big con to having it outside is that it becomes part of the global namespace. If the enum or related class only really applies to the class that it's in, then it makes sense. So in the printer case, everything that includes the printer will know about having full access to the enum PRINTER_TYPE, where it doesn't really need to know about it. I can't say i've ever used an internal class, but for an enum, this seems more logical to keep it inside. As another poster has pointed out, it's also a good idea to to use namespaces to group similar items, since clogging the global namespace can really be a bad thing. I have previously worked on projects which are massive and just bringing up an auto complete list on the global namespace takes 20 minutes. In my opinion nested enums and namespaced classes/structs are probably the cleanest approach.

DavidG
+1  A: 

paercebal said everything I would say about nested enums.

WRT nested classes, my common and almost sole use case for them is when I have a class which is manipulating a specific type of resource, and I need a data class which represents something specific to that resource. In your case, output_tray might be a good example, but I don't generally use nested classes if the class is going to have any methods which are going to be called from outside the containing class, or is more than primarily a data class. I generally also don't nest data classes unless the contained class is not ever directly referenced outside the containing class.

So, for example, if I had a printer_manipulator class, it might have a contained class for printer manipulation errors, but printer itself would be a non-contained class.

Hope this helps. :)

Nick
A: 

I agree with the posts advocating for embedding your enum in a class but there are cases where it makes more sense to not do that (but please, at least put it in a namespace). If multiple classes are utilizing an enum defined within a different class, then those classes are directly dependent on that other concrete class (that owns the enum). That surely represents a design flaw since that class will be responsible for that enum as well as other responsibilities.

So, yeah, embed the enum in a class if other code only uses that enum to interface directly with that concrete class. Otherwise, find a better place to keep the enum such as a namespace.

Pat Notz
A: 

If you put the enum into a class or a namespace, intellisense will be able to give you guidance when you're trying to remember the enum names. A small thing for sure, but sometimes the small things matter.

Mark Ransom
A: 

Visual Studio 2008 does not seem to be able to provide intellisense for nested classes, so I have switched to the PIMPL idiom in most cases where I used to have a nested class. I always put enums either in the class if it is used only by that class, or outside the class in the same namespace as the class when more than one class uses the enum.

Brian Stewart