CA1034: Nested types should not be visible

Question

Here's an explanation of the rule that that I am trying to understand. Here's the simplified code that the Code Analyzer was complaining about:

Public Class CustomerSpeed

    Public Enum ProfitTypeEnum As Integer
        NotSpecified = 0
        FlatAmount = 1
        PercentOfProfit = 2
    End Enum

    Private _ProfitTypeEnum As ProfitTypeEnum

    Public Sub New(ByVal profitType As ProfitTypeEnum)

        _ProfitTypeEnum = profitType

    End Sub

End Class

If the enum pertains only to the class, why is it a bad thing to make it a contained type within the class? Seems neater to me...

Does anyone know what is meant by the following line?:

Nested types include the notion of member accessibility, which some programmers do not understand clearly

Using Namespaces to group the Class and Enum doesn't seem like a useful way to resolve this warning, since I would like both the enum to belong to the same parent level as the class name.

Answer 1

I guess it indicates that nested type can be confused with static variable because they both will appear in intellisense (auto complete) after the ".".

Even if you see BCL (Base Class Library) of .NET, all the enums, even if they are only used in one class, they are not nested, reason is, when you want to compare or instantiate, the class names + "." + enum names will be confusing as shown below, also Microsoft is right that it will confuse people.

-- Example

class RootClass
{
   enum NestedEnum
   {
      StaticItem = 0     
   }

   static string NestedString = "";
}


RootClass.NestedEnum <-- represents enum,
RootClass.NestedString <-- represents static variable

One is a type and other is variable, most MS Code Analyzer warnings are for better design, however it has an big exception for @George, if you dont like it, dont use it, just go ahead and disable the warnings. You can certainly use big class names instead of namespaces, its simply your choice. But good programming skills is about how others percieve your code rather then what you like !!!

And namespaces are there to organize and by organizing we type less, thats what all about doing more work with less efforts. But if you like big names to type, no one is stopping you.

Answer 2

Enumerations are not usually placed in the class that uses them, so people are not used to having specify where the enumeration is:

Dim speed As New CustomerSpeed(CustomerSpeed.ProfitTypeEnum.FlatAmount)

Placing the enumeration outside the class makes it easier to use:

Dim speed As New CustomerSpeed(ProfitTypeEnum.FlatAmount)

The enumeration is still contained in the same namespace as the class. As the analysis explanation points out, you should use namespaces to group public members rather than nesting them in each other.

Answer 3

MS screwed up with the Code Analyzer. You make perfect sense George.

And by the way, my reputation isn't really 28, it just rolled over a couple of times.

Answer 4

In addition to the useability and discoverability issues already covered in other responses, there is also a potential maintainability issue. What happens on the day that you discover that your enum is also potentially useful elsewhere? Moving it out of its parent class would be a breaking change, copying it would introduce its own maintainability problems, and requiring API consumers to use it with another class starts to get ugly. Why not avoid these potential problems by systematically avoiding nested enums?