The class design rules form a group of style issues related to the class hierarchy and the definitions of classes and their members. This group of code review rules, part of the problem detection feature, detects potential problems with your VB.NET Classes, Structures and Interfaces. As these rules are related to programming style, not all people will agree on which style is the best. Pick the rules that suit your use.
This group of rules has no effect on classic VB classes. There are more class related rules in the Style group, for both classic VB and VB.NET.
The following rules apply to defining the correct constructors for .NET classes. See the appropriate rule for each class type.
We use the following terminology: Shared class = class that has only Shared members. Abstract class = MustInherit class. Concrete class = any other class.
Constructor missing. A concrete .NET class should define at least one constructor for instantiating objects of the class. The scope of the constructor should be used to define who can instantiate the objects. A Shared constructor is not enough for a concrete class. This rule skips abstract MustInherit classes and classes that have only Shared members. These special class types are subject to the rules Protected constructor expected and Private constructor expected, respectively. CTOR
Protected constructor expected. Constructors of an abstract MustInherit class in .NET can only be called from an instantiating subclass. Marking all constructors Protected helps indicate this. Having no constructors in an abstract class is also a viable alternative. This rule ignores Shared constructors. CTOR
Shared constructor decreases performance. Having a Shared Sub New makes the program run slower. Even though the shared constructor executes only once, its execution is checked every time a Shared member is accessed or when an instance of the type is created. There is also the problem that you have no control over when the shared constructor really runs. It is better to declare Shared variables with an initializer such as Shared myvar = 7. This guarantees the variable will have the value without overhead, whatever happens in your program. Note that a Sub New in a Module also counts as a shared constructor. SHARED_CTOR
The following set of rules applies to finalization of .NET classes. The destructor Sub Finalize executes when a object has been disposed of and the .NET run-time garbage collector (GC) is about to free the object. You can use Project Analyzer to set requirements for your destructor strategy. The alternatives are the following:
Finalize found. A .NET class defines a Finalize destructor. Finalization is generally not required if the class uses only .NET managed resources. The existence of a Finalize destructor will slow down the garbage collection process, which has a negative performance impact if a large number of instances of a class are created and destroyed. For maximum speed, finalization should be left to classes that allocate unmanaged resources. FINALIZE
IDisposable not implemented. A .NET class defines a Finalize destructor but does not implement the IDisposable interface. An object that allocates some resources should implement IDisposable.Dispose to free the resources when the object is no more required. Users can call Dispose to free the resources as soon as possible. Relying on the Finalize destructor to release resources will lead to resources staying allocated for a longer time than what is necessary. This happens because the garbage collection process will execute Finalize asynchronously at a later time. Read .NET help for 'Implementing a Dispose method' for information on how to implement finalization with IDisposable. See also the rule SuppressFinalize missing. NO_IDISPOSABLE
Finalize missing. A .NET class defines no Finalize destructor. Proper finalization is required if the class uses unmanaged resources. Finalization is generally not required if the class uses only .NET managed resources. The compulsory use of a Finalize method is a matter of programming style. NO_FINALIZE
Finalize missing MyBase.Finalize. A Finalize destructor of a .NET class should call MyBase.Finalize to free up any resources allocated by the base class. If the class derives directly from System.Object, this call is not necessary. NO_MYBASE_FINALIZE
SuppressFinalize missing. A method implementing IDispose.Dispose should call GC.SuppressFinalize(Me). The Dispose method should make this call to prevent the Finalize destructor from getting executed by the garbage collection process. If the Dispose method frees all allocated resources, it is unnecessary for the GC to call Finalize to free them again. By calling GC.SuppressFinalize(Me) you make the garbage collection run more efficiently. This rule applies to .NET classes implementing the IDispose interface and having a Finalize destructor. NO_SUPPRESSFINALIZE
The following few rules deal with the declaration of class methods, variables and so on.
Shadows keyword found. The Shadows keyword in VB.NET allows a child class to hide members of its ancestor classes. Shadowing is not recommended as it makes programs harder to understand. It may not be clear to a developer that there are two different things with the same name. See also the rule Child reuses ancestor member name. SHADOWS
Child reuses ancestor member name. A child class member has the same name as a private member of its ancestor class. Even though a child class is allowed to have any member names, it should not reuse the names of private members of its ancestors. The use of similar names increases the probability of programming errors. First, it may not be clear to a developer that there are two different things with the same name. Second, if the ancestor member scope is later changed, it will result in name shadowing. This rule is related to the Shadows keyword but technically speaking, it is not actual name shadowing because a private ancestor member is not visible to the child. REUSES_NAME
Member scope exceeds container scope. The scope of a member is wider than the scope of the class, module or structure that it belongs to. A member cannot have higher visibility than its container. Although this causes no immediate faults, it may make the code harder to understand and more prone to errors. If the scope of the container is changed later, the members may accidentally have higher visibility than desired. Notice that the scope of the container may be set automatically by VB (if no scope is defined) or it may be further limited by the container of the container. This problem is shown if the member explicitly defines too wide a scope. It is not shown if the member has implicit scope declaration, that is, no scope keyword. As an exception, if a procedure is declared with the Overrides keyword, its scope cannot be defined freely and is thus not subject to this rule. Member scope checking is available for VB.NET only. In VB Classic, scope choices are limited and not likely to cause confusion. MEMBER_SCOPE
Protected found in NotInheritable class. The use of a Protected or Protected Friend scope for a member of a NotInheritable class makes no sense as there cannot be any subclasses. Rewrite Protected as Private and Protected Friend as Friend. As an exception, Protected and Protected Friend may be required in an Overrides declaration. PROTECTED
Shared expected. A Sub, Function or Property does not access any instance variables or procedures. It can be declared as Shared. In addition to determining missing Shared keywords, this rule helps you spot procedures that do not access instance variables even when they should. After adding the Shared keyword to some procedures, re-analyze the program to see if any callers of the new Shared procedures should also be Shared. NO_SHARED
Non-instantiatable class has instance members. A class cannot be instantiated, because it has only Private constructors. Only the Shared members of the class can be accessed. The instance members are inaccessible and will remain unused. Reconsider the design of this odd class. One of the constructors could be made accessible, or the instance members could be removed or moved to another class. Alternatively, the class could be instantiated from an inner Class or Structure. NONINST
Class/Structure nested inside Interface. Interfaces should not define inner classes or structures. This is bad programming. An interface is an abstraction of a concept, a Class or Structure is an implementation. Don’t mix these, but leave the implementation details out of interfaces. Nesting an Interface within an Interface is odd as well, but since it may have special uses, this rule does not trigger a warning about them. IN_INTERFACE
Interface ambiguous. An Interface defines the same name twice. This happens when an Interface inherits two base Interfaces having methods or properties with the same name. Calling the duplicated member is cumbersome. To avoid an ambiguous call, type casting is required. Consider renaming one of the members. Alternatively, rewrite the interface hierarchy. This rule does not apply to method overloading, which is a non-ambiguous way to reuse a name. INTERFACE_AMBIGUOUS
Interface duplicated. There is a superfluous interface. An Interface inherits the same base Interface twice. Alternatively, a Class or a Structure implements the same base Interface twice. While it causes no real harm, consider clarifying the interface hierarchy so that duplication is not necessary. INTERFACE_DUPLICATED
The following rules deal with the use of a class as a parent class.
Parent class instantiated. An instance of a .NET parent (base) class is created. Keeping parent classes abstract and instantiating only leaf classes will make your class inheritance tree more clear. See also the rule Parent class requires MustInherit. PARENT_INST
Parent class requires MustInherit. Add MustInherit to all your .NET parent (base) classes to prevent accidental instantiation. Keeping parent classes abstract and instantiating only leaf classes will make your class inheritance tree more clear. See also the rule Parent class instantiated. NO_MUSTINHERIT
Inheritance limited. All methods and properties of a class are sealed but the class is not. The class can be derived but none of the derived methods or properties can be overridden by subclasses. Subclasses can thus add new functionality but they cannot modify existing behavior. This restriction can be considered a form of limited inheritance, indicating a possible design flaw. You have two choices to clear things up. If the class is not designed to be derived from, prohibit inheritance by adding a NotInheritable keyword to the class and removing any NotOverridable keywords from the methods. This way you explicitly seal the class to disallow limited inheritance. Alternatively, go from limited inheritance to more versatile inheritance by defining some appropriate methods Overridable. You can also choose to overlook this problem if limited inheritance is what you want to achieve. INHERITANCE_LIMITED
Parent class inherited only once. A parent class has only one immediate child class. The inheritance hierarchy seems unnecessarily deep. In general, a parent class should be used to derive two or more child classes, while not creating instantiations of the parent. Since this parentclass has just one child, it's like an extra layer of inheritance. The hierarchy is not as simple as it could be. Consider joining the parent with the child. Determine if the parent class was designed to prepare for future expansion of the class tree. If so, add the missing child class. This rule applies to VB.NET classes only and it ignores Interfaces. INHERIT_ONCE
An abstract class is one that doesn't allow instantiation. In VB.NET, the syntax to declare a class abstract is to add the MustInherit keyword. Instantiation is allowed for concrete classes that Inherit the abstract class.
MustInherit class missing MustOverride methods. An abstract class (MustInherit) does not define any abstract methods or properties (MustOverride). Why is the class abstract then, really? An abstract class defines an incomplete implementation. Child classes complete it by implementing the abstract methods. Reconsider the design of this class hierarchy. NO_MUSTOVERRIDE
MustInherit class inherits concrete class. An abstract class (MustInherit) is derived from a concrete class other than System.Object. Abstract classes should be on top of the class hierarchy. The hierarchy should not be unnecessarily deep either. MUSTINHERIT_CONCRETE
MustOverride overrides concrete procedure. A MustOverride abstract procedure overrides a concrete procedure. The class deletes inherited functionality, but does not provide any replacement. Is this correct? MUSTOVERRIDE_CONCRETE
Class looks like Interface. An abstract .NET class looks like an Interface in that it defines actions but does not implement them. Could you rewrite the class as an Interface to make this explicit? If the only purpose of the class is to define a set of procedures for descendants to implement, redefining it as an Interface might be more appropriate. It is a matter of programming style which way you prefer. This rule finds abstract MustInherit classes that define one or more empty procedures. In addition, they must not define any implementation, that is, variables, Private procedures or any procedures with code in them. If the class inherits another class (other than System.Object), the parent must also look like Interface (except that it does not necessarily have to define any procedures). LIKE_INTERFACE
This group of rules deals with classes that have only Shared members. Such a class is called a static class. All instances of the class are exactly similar. Different instances do not have any individual state variables.
Note that a Module is a special case of a Shared class, really. These rules don't affect Modules, though.
NotInheritable expected. A class has only Shared members. It was probably not designed for use as a base class for Inherits. Mark it NotInheritable to make it explicit and to prevent accidental inheritance later. This rule ignores classes that have already been inherited from. NO_NOTINHERITABLE
Shared-only class instantiated. A class with only Shared members is being instantiated. There is no need to create an instance of this class. You can access the Shared members through the class name as a qualifier. Ideally, you should declare a Private Sub New constructor for the Shared-only class to prevent accidental instantiation. SHARED_INST
Private constructor expected. A .NET class that has only Shared members should define a Private Sub New() to prevent instantiation. There is no value in creating an instance of a class that contains only Shared members. To prevent such extraneous instantiation, ensure that the class has a single, no-argument, private constructor and no other constructors. This rule applies to .NET classes having only Shared procedures and variables. Sub New and Finalize are not taken as procedures. CTOR
The rules on this page are covered by the comment directive parameter STYLE.