9

As noted by the title, is nesting types (e.g. enumerated types or structures in a class) considered bad practice or not? When you run Code Analysis in Visual Studio it returns the following message which implies it is:

Warning 34 CA1034 : Microsoft.Design : Do not nest type 'ClassName.StructueName'. Alternatively, change its accessibility so that it is not externally visible.

However, when I follow the recommendation of the Code Analysis I find that there tend to be a lot of structures and enumerated types floating around in the application that might only apply to a single class or would only be used with that class. As such, would it be appropriate to nest the type sin that case, or is there a better way of doing it?

Improve this question
2
  • This question is VERY closely related to programmers.stackexchange.com/questions/34067 I'm torn on whether to close this as a duplicate...
    – Walter
    Jan 11, 2011 at 15:20
  • @Walter - They are close but I've heard the question brought up separately for nesting classes in classes and nesting all other types in classes separately so it couldn't hurt. As it turns out, I didn't find that question when I did my search originally.
    – rjzii
    Jan 11, 2011 at 15:28

4 Answers 4

Reset to default
16

Nested types are not bad. The warning you are receiving is not suggesting you never have a nested type. It is simply indicating that your nested type should use an appropriate access modifier and code location.

If the nested type is truly only used inside the containing class (i.e. it is an internal data container or status indicator), then set its access modifier to private.

If the nested type is part of one or more method signatures, then it is not in fact local to the containing class. It represents a message being passed to or from instances of the containing class. In this case, it is arguably better to move the nested type out of the containing class, and give it a more open access modifier such as internal or public.

In short, the warning appears to be recommending that you make local types private, and shared types should stand alone.

Improve this answer
1
  • The internal modifier is what I've been doing in most cases, cleans things up a bit, but there are a lot of them floating around in the application.
    – rjzii
    Jan 11, 2011 at 15:29
2

If it is used solely by that class it should be made private and that is exactly what the message is suggesting.

Improve this answer
1
  • In some cases that is done, but in other cases an enumerated type is being used in method calls attached to the class. Likewise, I've also seen structures being used for method class if the number of parameters exceeds a given number.
    – rjzii
    Jan 11, 2011 at 14:42
1

The warning about nesting types was one of the first "suggestions" I received some time back after having enabled the Code Analysis. It was also the reason why I switched it off.

Some suggestions are really foreign as though they come from another planet.

I do put enumeration inside classes to keep things logically together.

Think of it this way: if nesting types would have been absolute evil in all cases, why would the language designers have implemented it in the first place? Because it is useful in many cases and in that regard the said warning is merely a pointing out that something in your code might not be optimal. If it applies or not is up to you to decide.

Improve this answer
6
  • 2
    A namespace should be your logical grouping for the enum; not the class
    – Aaron McIver
    Jan 11, 2011 at 14:53
  • 1
    Well in some situations I find it inconvenient.
    – user8685
    Jan 11, 2011 at 14:54
  • If you are currently placing the enum within the class, moving it outside of the class so it resides within the namespace takes all of a CTRL+X coupled with a CTRL+V...not sure how it would become inconvenient?
    – Aaron McIver
    Jan 11, 2011 at 15:03
  • I sometimes put my enums inside classes. I think there are legitimate reasons to it - its not a strict rule.
    – Nobody
    Jan 11, 2011 at 15:18
  • 3
    @rmx I have never used an enum within a single class; it is always used within multiple classes, so defining it within the namespace as its own entity makes much more sense.
    – Aaron McIver
    Jan 11, 2011 at 16:00
0

No

A good example in C# is IEnumerable where other classes doesn't need to know the exact class it returns, only that it is a IEnumerator. So it makes sense to make it an nested class otherwise you might have lots of small classes floating around in intellisense that implements IEnumerator

Improve this answer
1
  • In some cases, run-time efficiency can be improved if a compiler knows the type returned by GetEnumerator(); for that reason, List<T>.GetEnumerator() returns List<T>.Enumerator(). If had been some way to ask the compiler to use some other named method, it might have been better to have that method e.g. ForEachGetEnumerator() return List<T>.Enumerator and have GetEnumerator() return IEnumerator<T>, but no mechanism exists for that.
    – supercat
    Mar 19, 2014 at 20:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.