Code review rules
Style suggestions are code review rules by which you can add legibility and understandability to your code and also increase its quality. The style rules don't necessarily have an effect on the compiled program, they focus on the quality of the source code instead. These rules are very useful if you want to enforce programming standards in your work and across teams.
As the title implies, the style rules are a matter of coding style. You are encouraged to ponder the feasibility of each rule regarding your current programming style and standards. Configure your own problem filter settings and choose the rules that suit you best.
Uncommented procedure. A procedure is not commented. It may be hard to understand what the procedure does, what parameters it takes and what the return value is. The internal functioning of the procedure isn't commented either. Use this rule to require at least one comment in each procedure. — You can ignore short uncommented procedures with a maximum of x lines of code. Write x in the 'Ignore when' textbox in Problem Options. Lines Sub/Function and End Sub/Function are counted as code, making an empty procedure have 2 lines. Leave the box empty if you don't want to have a limit. NO_COMMENT
Too many uncommented lines. Several code lines were found without any comments in them. Add enough comments to make the code easier to understand. This rule finds consequtive uncommented lines. You can set the 'too many' limit to your taste. The rule counts the lines between comments. Empty lines are skipped. Lines continued with the line continuation syntax ' _' count as one line. The lines are counted for each procedure individually so that small uncommented procedures will not trigger an error even if they exceed the limit together. This rule is useful for finding long uncommented blocks inside procedures that have some comments, but not enough. It differs from the 'Uncommented code' rule in that this one reports any consequtive lines without comments, while the 'Uncommented code' rule targets entire procedures with zero comments. NO_COMMENT
Trailing comment found. An endline comment has been found after code. Comments should be written as the only thing on a line (full-line comments). As the space to the right of a line is limited, endline comments tend to be cryptic. There simply isn't enough space to write a good comment. It's better to write a descriptive full-line comment, on one or more lines, before the appropriate block of code. Trying to shortly comment each line as you go does not necessarily tell what the code is really intended to do. In addition, endline comments should look aligned and not wavy. Aligning them takes time, especially when the code is later changed. One should spend more time writing new comments, not adjusting old ones. TRAILING_COMMENT
Rem comment found. The Rem comment syntax is obsolete. In modern code, comments are marked with the single quote character ('). REM
Possibly commented-out code. The line(s) may contain commented-out code. Removed code that has been left as a comment may be considered junk. It may distract developers and cause confusion about its significance. It's possibly a bad or otherwise outdated piece of code. Consider removing commented-out code to clean up your project. Project Analyzer uses heuristic rules to search for comments that resemble the actual code in the project. The rules are not foolproof, though, and a certain deal of false alarms are to be expected. In order to mark a line as being commented out on purpose, you can prefix it with a dollar: '$If fail Then End. The dollar will make the line look like non-code to Project Analyzer, thus excluding it from the check. COMMENTOUT
Line too long. A line of code is longer than allowed. A typical limit is 80 characters per line, but you can also use higher limits such as 90 or 100. Long lines are hard to read and understand. Overly long lines don't fit in the view of your development environment and you need to scroll. What fits in view depends on the monitor and font. On a 1280x1024 screen you can reasonably allow about 80-90 characters in a 10pt Courier New font and maybe 100 characters in an 8pt font. Long lines won't fit on paper either, and need to be wrapped. While the Print feature of Project Analyzer wraps neatly in a syntax-aware fashion, other programs might not. As an additional bonus, enforcing short lines discourages too deep nesting. How to shorten the lines? Split complex expressions into two or more statements with temporary variables. This will make your code more self-explanatory. Splitting a line with the line continuation character (_) is only a partial answer. It's like manual word wrapping. A long statement is not any easier to understand even if it covers several lines. LONG_LINE
Multiple statements on line. There are more than one statement on a single line, separated with a colon ( : ). To make your program more readable, write only one statement on one line. MULTISTATEMENT
Magic number found. A magic number is a hard-coded numeric literal that is not defined as a constant. Unnamed magic numbers in code are obscure as they do not reveal the meaning of the number. They increase opportunities for subtle errors and make it more difficult to extend or modify the program in the future. Replacing magic numbers with named constants makes programs easier to read, understand and maintain. The same value is often duplicated in different places of the program. Changing such a value is error-prone as each location must be considered separately. In addition, if two distinct magic numbers have the same value, they may be accidentally edited together, even though only one of them should have been changed. The following decimal integers are not considered magic: -1, 0, 1, 2. These numbers are frequently used as initial values, For loop control values and in common mathematical expressions. A hexadecimal or octal zero is not considered magic either. MAGIC
Magic decimal number found. A number with a decimal fraction is not defined as a constant. See Magic number found for a full description. MAGIC, MAGICDEC
Magic hex number found. A hexadecimal number is not defined as a constant. See Magic number found for a full description. Exception: &H0 is not considered magic. MAGIC, MAGICHEX
Magic octal number found. An octal number is not defined as a constant. See Magic number found for a full description. Exception: &O0 is not considered magic. MAGIC, MAGICOCT
Octal number found. An octal number has been found. Octal numbers are expressed in digits 0-7. Since octal numbers are relatively rare, they can cause problems reading the code. To avoid confusion, express numeric values in either decimal or hexadecimal format, which are more widely understood. OCT
Fixed file number found. Use of fixed file numbers (hard-coded integers) in file handling is a potential cause of run-time errors. It is better to use a variable and initiate it with the FreeFile function like this: FileNum = FreeFile. FIXED_FILENUM
Enum has implicit member value(s). One or more members of an Enum do not have an explicitly declared value. The values might change if new members are added. This potentially affects code where the old values are used instead of the enum member names. IMPLICIT_ENUM
Enum missing zero value. An Enum should define a constant with the value of zero. The default value of an uninitialized Enum variable is zero. Explicitly defining a zero Enum constant makes the uninitialized value valid. You can name the zero constant with a descriptive name such as None or Empty or Error. ENUM_ZERO
Enum with duplicate value. Two or more constants in an Enum define the same value. Check to see if this is by accident. Declaring the same value several times may be intended. On the other hand, an error may have caused the duplicate definition. Check to see if you can join the multiple constants into one to prevent problems understanding the Enum later. ENUM_DUPLICATE
Enum constant expected. An enumeration constant is expected on this line. An Enum datatype is assigned a value not belonging to that Enum. One should only store Enum values, not integers or other datatypes. Otherwise you easily end up with a value outside to the Enum. An Enum should fully define and document the datatype's acceptable values. Using an undefined value is undocumented programming, really, or even an indication of erroneous logic. A programming style that uses 'magic' integers interchangeably with Enum constants is prone to maintenance errors. If the Enum values are later changed, the respective 'magic' integers are easily left unchanged, which leads to new errors. This rule checks assignment statements and the default value of an Optional parameter. In addition, it checks Enum-type Consts. The warnings triggered by this rule are for your information, not necessary an indication of a real problem. At times, you may choose to ignore this warning when the code is clear or when it is not possible to rewrite it. ENUM_CONST_EXPECTED
File with several classes or modules. Each file should contain just one class or one module. This makes it easier to keep track of the project structure. It's OK to put related Structures or Interfaces in the same file with a class or module. It's also OK to nest classes when necessary. This rule applies to VB.NET only. MANYMOD
Microsoft.VisualBasic.Compatibility imported. Use objects and methods in the .NET Framework and stay clear of those defined in the Microsoft.VisualBasic.Compatibility namespace. The Microsoft.VisualBasic.Compatibility namespace is used by the tools that upgrade Visual Basic 6.0 code to Visual Basic .NET. It is a bridge to support Visual Basic 6 features that are not directly supported by the .NET implementation of Visual Basic. Microsoft.VisualBasic.Compatibility adds a layer of complexity to your application and introduces some minimal performance costs. The native .NET methods are often faster and offer more options. COMPATIMPORT
Return statement found. A Return statement causes an immediate jump out of a procedure in VB.NET. It potentially makes the procedure have several exit points. Procedures should have only one exit point at the end. — There is a case where VB requires a premature Return. This is when you need to quit a procedure immediately before an On Error Goto xxx type error hander. Another case where Return must be used is in an Operator (VB 2005 and later). Project Analyzer allows these uses. This rule is available for VB.NET. RETURN
Multiple Return statements found. A procedure contains multiple Return statements. There should be exactly one exit point in a procedure. Multiple exits make the procedure harder to understand. The requirement for multiple exits may indicate too complex a procedure, in which case it should be split. Rewrite the procedure to contain a maximum of one Return statement. This rule applies to VB.NET. Note that this rule does not count Exit Function statements. The rule 'Exit Sub|Function|Property statement found' detects them. MULTIRETURN
On Error style used. A procedure utilizes On Error statements for error handling. On Error is old syntax in VB.NET. It is better to use the Try..Catch block for error handling. On Error Resume Next also deteriorates the performance of .NET execution. Use Try..End Try without the Catch block to achieve a similar effect without the performance hit. ONERROR
Here you find a set of statements whose use is questionable in some way. You can mark any of them disallowed and get notified if they are found in your code.
GoTo statement found. A GoTo or On..GoTo statement exists in code. Use of GoTo is bad programming practice and should be avoided when possible. Jumping around with GoTo easily leads to unstructured control flow structure and spaghetti code. GOTO
GoSub statement found. A GoSub or On..GoSub statement exists in code. Use of GoSub is bad programming practice, leads to low execution performance and should be avoided when possible. In addition, VB.NET doesn't support GoSub. Rule applies to VB 3-6. GOSUB
Call statement found. The Call statement is no longer needed. You may remove it. CALL
Let statement found. The Let statement is obsolete. Instead of Let x = 1, you can simply write x = 1. LET
IIf / Switch / Choose function found. These functions are considered bad programming style because of functionality, optimization and readability issues. All conditions and branches of IIf / Switch / Choose are always executed resulting in longer execution time and possible unwanted side-effects. Switch and Choose may return an unwanted Null. Use Select Case or If..End If instead. IIF, SWITCH, CHOOSE
On Local Error statement found. The 'On Local Error' statement can be rewritten as 'On Error'. The Local keyword is redundant. All error handlers are local to the procedure regardless of the keyword. ON LOCAL ERROR
While loop found. The While loop is outdated. Do...Loop provides a more structured and flexible way to perform looping. In VB 3-6, the syntax to avoid is While..Wend. In VB.NET, it is While..End While. WHILE_WEND
Single-line If..Then statement found. An If..Then(..Else) construct is on a single line. To make your program more readable, split the construct to multiple lines. Use indentation like this:
If condition = True Then true-part Else false-part End If
Besides, the more often executed branch should be the true-part, because it is faster that way. SINGLE_LINE_IF
Exit statement found (Exit For|Do|While etc.). Use of the Exit statement indicates unstructured program flow, much the same way as the Goto statement. An Exit statement causes an immediate jump out of a programming structure such as a loop. In purely structured programming style Exit statements should not be used. Well-placed Exit statements are not considered harmful by many programmers, though. EXIT
Exit Sub|Function|Property found. An Exit Sub|Function|Property statement causes an immediate jump out of a procedure. It potentially makes the procedure have several exit points. Procedures should have only one exit point at the end. — There is a case where VB requires a premature Exit. This is when you need to quit a procedure immediately before an On Error Goto xxx type error hander. Project Analyzer allows this use. EXIT
For with Step 1 found. Specifying Step 1 in a For statement is redundant because one is the default value. You may omit the Step 1 clause. STEP1
Next statement ends multiple For loops. A Next statement with two or more variables was found. The statement ends more than one For loop. For clarity, terminate each For block with its own Next statement. This way the For and Next statements will appear in pairs and make the code easier to understand. MULTINEXT
The following rules are for the proper declaration of data types.
Variable/function missing type declaration. A variable does not have a defined data type. By default, the type is Variant in classic VB and Object in VB.NET. These generic types need more memory than other types. Decide what type you need and write it to the declaration. As an additional bonus, upgrading to VB.NET will be easier if you use explicit data types. In VB.NET, you can set Option Strict On to require explicit data types. NO_TYPE
Constant missing type declaration. A constant has no defined data type. VB will pick an appropriate data type for it. The chosen type may not be optimal where the constant value is actually used. You can use this rule to require explicit typing of constants. NO_TYPE
Function/Property with type character. A Function or a Property is declared with one of the old-style $#%!&@^ type characters. Instead of MyFunction$( ), use MyFunction( ) As String, etc. TYPE_CHAR
Variable/parameter/constant with type character. A variable, procedure parameter or constant is declared with one of the old-style $#%!&@^ type characters. Instead of Year%, use Year As Integer, etc. Note: The VB.NET Nullable character ? is not considered to be problematic. TYPE_CHAR
Variable/Parameter with generic type. A variable or parameter is declared as a generic type. While this is completely legal, it may lead to slow and error-prone code. VB may have to use 'late binding' because it doesn't know what kind of object to expect. In late binding, calls to the object's members are slower and may result in a run-time error if the member doesn't exist. If you know what kind of an object the variable will contain, declare it as that type. This is especially important if you plan to migrate existing VB 3-6 code to VB.NET. The .NET upgrade wizard cannot successfully convert all late-bound calls, resulting in manual work and errors. GENERIC
Fixed-length string used. The fixed-length string data type should be reserved for uses where it is required. In many cases, the variable-length string provides more efficient memory usage and better performance. Use of variable-length strings is also preferred if you plan to convert to VB.NET later as VB.NET provides only limited support for fixed-length strings. This problem is available for VB 3-6. FIXED_LEN
DefType statement found. DefType statements force data types on declarations starting with the given letters. Relying on DefType may cause confusion because the datatype is not visible on each declaration. Use the 'As Datatype' syntax for more readable declarations that are also easier to maintain. The rule catches the following statements: DefBool, DefByte, DefInt, DefLng, DefLngLng, DefLngPtr, DefCur, DefSng, DefDbl, DefDate, DefStr, DefObj, DefVar. This problem is available for VB 3-6. DEFTYPE
These rules trigger only when Option Infer is On. Option Infer became available with VB2008. It lets one leave out the "As Datatype" part from declarations, as well as declaring For index variables automatically without any declaration syntax at all.
Type inference used. A local variable has no explicit datatype definition. The compiler infers its datatype from the variable's initialization value. A feature introduced in VB2008, this happens with the setting Option Infer On. Since a variable may be intended to contain datatypes other than its initial value, each variable should preferably be declared with an explicit datatype. There is also a maintenance issue to consider. The inferred datatype may depend on a function call or a reference to other data. If the datatype of that reference is later changed, the change is propagated to this variable too. While this may be desirable, it may also introduce a hidden bug where the variable is then used. In other words, the variable's datatype may be subject to an unintentional change. Adding an As datatype clause makes the datatype explicit and prevents unwanted changes. INFER
Implicit variable. This statement creates a new local variable without any explicit declaration. This can happen with the For and For Each statements with Option Infer On (introduced in VB2008). While this causes no immediate problem, there is a risk of maintenance errors. If a variable with the same name is later Dim'd, the statement will start changing that variable, which is most probably not wanted. Fix the problem by explicitly adding a datatype declaration with syntax For [Each] j As datatype. You can switch Option Infer Off to prevent this problem from reoccurring. IMPLICIT_VAR
This is a set of rules for various declarations and keywords.
Scope declaration missing. A part of your program does not have a defined scope. Instead, it is using a default scope as given by VB. VB's default rules are somewhat complicated and may lead to a scope that is too wide or too narrow. With a narrow scope, you can encapsulate functionality and data. With a large scope, you give access to this part of your program from other parts. Determine which scope is appropriate and declare it explicitly. The current default scope is given. This problem is not reported for VB 3.0, which has a limited scope declaration syntax. NO_SCOPE, SCOPE
Excess scope. A part of your program has a wider scope than necessary. It is a good practice to use as tight a scope as possible to prevent other parts of the program calling and modifying parts that they shouldn't have anything to do with. You can define a more limited scope without affecting the current use of this code. Before limiting the scope, see if there is a reason why the wide scope might be necessary later. This problem type is currently available for analyses containing only VB 3-6 projects. EXCESS_SCOPE, SCOPE
Global found, use Public. Use of the Global keyword is outdated. Simply replace Global with Public. VB3 supports only Global. VB 4-6 support Global but favor Public. VB.NET no longer supports Global but requires Public. GLOBAL, SCOPE
Global variable found. Global variables (Public and Friend variables declared inside standard modules) are convenient for programmers working under pressure. It's usually easier and faster to define globals rather than pass parameters. Unfortunately, as the global variable is fully accessible from all parts of the project, it's difficult to determine the source of error should the global variable be found to have an incorrect value. This has an effect on debugging time. PUBVAR
Encapsulate non-private variable as property. Variables that aren't Private should be avoided in classes. Read/Write access to a class's member variables should only be allowed via properties. Make the variable Private and add a property to read/write the contents. PUBVAR
Warning Passed ByRef. Encapsulated Property will not receive changes back from ByRef. In classic VB, passing a variable ByRef may change the value of the variable. Passing a Property ByRef will not change the value, however. This can lead to errors that are hard to debug. When you encapsulate a variable as a property, make sure the logic still functions with ByRef calls. Project Analyzer detects ByRef passing and shows the warning text for the problem. This is not an issue with VB.NET because VB.NET uses copy-in/copy-out semantics for Properties passed ByRef, so variables and encapsulated properties work the same.
Variable/constant declared in code. A Dim, Static or Const statement is found among procedure code. You can require that all local variables and constants be declared in their own block at the start of a procedure. The benefit of this style is that all declarations can be found quickly in one block. Note that the use of this rule disallows block-scope variables in VB.NET. DECL_IN_CODE
ReDim without Dim. A ReDim statement allocates a local array without a prior Dim declaration. It is equivalent to using undeclared local variables without Option Explicit. An exceptional statement in classic VB, ReDim is not controlled by Option Explicit. There is a risk of maintenance errors. If an array with the same name is later Dim’d outside this procedure, the ReDim will erase and reallocate that array, which is most probably not wanted. Fix the problem by adding a local declaration with syntax Dim() arrayname As datatype above the ReDim line. This will protect you from an accidental change to a non-local array. Pay attention to declaring the correct datatype to preserve memory. This rule applies to classic VB only. REDIM
Initializer missing for variable. A variable declaration does not contain an initializer. The default value for the variable will be zero or Nothing. A value of Nothing can lead to a run-time error when used. You can initialize a variable by either of the following syntaxes: 'Dim x = y' or 'Dim x As New ...'. The latter option is better as it fixes the data type of the variable as well, whereas the previous option does not necessarily do so (depending on VB version and project options). This rule allows you to require explicit variable initialization in VB.NET. Since instance fields of a Structure cannot have initializers, the rule ignores them. As an exception, initializers are required for Shared fields of a Structure. NO_INITIALIZER
Initializer missing for local variable. This is the same rule as Initializer missing for variable, but affects local variables instead of Class, Structure or Module-level variables. NO_INITIALIZER
ReadOnly variable expected. A variable is assigned a value in its initializer or a constructor, but nowhere else. Declare the variable as ReadOnly to make it explicit that the value does not change. This prevents accidental writes to the variable. Note that ReadOnly does not provide 100% security in terms of preventing malicious programs from changing the value. ReadOnly can be added to non-local variables in VB.NET. NO_READONLY
NameCheck. A name fails Project NameCheck. Fix the name or select another naming standard. NAMECHECK
Default name. A project, module or control has not been named with a descriptive name. VB has generated a default name instead, like Project1, Module1, Text1, List2 etc. This rule is available for VB 3-6 controls and VB 3-6 and VBA project and module names. DEFAULT_NAME
Short name. A name is too short and not likely to be descriptive. The name can belong to a control, procedure, variable, constant, Type or Enum—just about anything. Names should be descriptive to keep the code easier to understand. You can the limit the length by yourself. Project Analyzer reports names that are equal to or shorter than your limit. You can optionally allow single-character local variables (i, j, k...) used as For index variables. This rule allows numeric x and y as parameters and local variables, as these names are frequently used for coordinates, but only when they exist as a pair, declared next to each other. This rule allows the following names in VB.NET: e As EventArgs and Catch ex [As Exception]. SHORT_NAME
Option Explicit Off. Option Explicit is not set for a file, or it is deliberately set Off. You should always use Option Explicit. It makes VB require explicit variable declaration. This way you avoid using unnecessary implicit Variants (classic VB) or Objects (VB.NET). Even better, you get rid of some nasty errors caused by typing errors and variables with similar names. The use of Option Explicit will also help in upgrading code to VB.NET. By forcing explicit variable declaration you prevent accidental use of late binding, which is harder to upgrade. MISSING_EXPLICIT, EXPLICIT_OFF
Option Strict missing. Option Strict is Off for a VB.NET file. There is no Option Strict statement in the file and Option Strict is Off in project settings. Use this rule to require Option Strict On. In VB.NET, Option Strict On enforces type-safe code by allowing only widening data type conversions. It also disables late binding. You can set it On either by writing Option Strict On in the file or changing the Option Strict setting in project options. This rule allows an explicit Option Strict Off statement. Use it as an exception in those files that require it. MISSING_STRICT
CreateObject found. CreateObject is a late-bound call. Generally speaking, you should use early binding to achieve better performance and also to avoid programming errors. To replace CreateObject with an early-bound call, simply use the syntax As New Classname. If this is not successful, add a reference from your project to the underlying COM library. This rule allows the use of CreateObject when creating objects on a remote server. CREATEOBJECT
Undefined compiler constant. A compiler constant was found, but no definition for it. While this is perfectly legal, it could also indicate a problem, such as a mistyped condition or accidental removal of the definition. COMPCONST, UNDEF_COMPCONST
For index variable not local. A For or For Each loop should use a local index variable. This index variable is defined outside the procedure. Alternatively, it is a ByRef parameter. The loop updates the variable's value, which may be an unexpected side effect in this procedure. Fix the problem by declaring a local variable of the same name. In VB2003 and later, use the syntax For j As Datatype to create a local loop variable. In VB2008 and later, this problem can accidentally occur with Option Infer On. FORVAR
Interface members missing. A class, interface or a VB.NET Structure does not define any members to access from other parts of the code. There is no way to execute methods or pass data. This could indicate a design flaw: some code is possibly missing. This rule warns about marker interfaces and interfaces that only define constants. The following definitions count as interface members: Sub, Function, Property, Operator and variable (other than Private). A class may also use the Implements keyword to allow access via an external interface. Inherited members do not count as interface members, nor do DLL procedure declarations. This rule works for both VB.NET and classic VB classes. NO_INTERFACE
Data-only class found. A class consists only of data. There is no behavior. Is this a class, really? Consider whether the class would work better as a user-defined Type or Structure. You may also want to put the variables as member variables of another class or classes. As exceptional cases, this rule allows data-only interfaces (classic VB .cls interfaces) and data-only base classes with at least 2 children. The latter case is a common data class with different implementation classes that act on the same data. DATAONLY
Dataless class found. This class contains procedures, but no class-level variables. Check to see if this is the right way to obtain abstraction. The procedures might work better in a module or as a part of another class. There is no point in instantiating dataless classes, really. If you want to keep the dataless class, consider making it a static class with Shared methods and a Private Sub New to prevent instantiation. This rule explicitly allows dataless Shared-only classes. DATALESS
Interface class instantiated. A VB5/VB6 .cls file defines an interface that is being implemented by another class, yet the class is also instantiated. Keeping interface classes and concrete classes separate will help in clarifying the program's structure. An interface class should be pure in that it contains no executable code and that it is not instantiated. See also the rule Interface class contains code. INTERFACE_INST
Interface class contains code. A VB5/VB6 .cls file defines an interface and also contains executable code. The class's interface is being implemented by another class, yet there is code in the class's procedures. It is a mix of interface definition and concrete code. Keeping interface classes and concrete classes separate will help in clarifying the program's structure. An interface class should be pure in that it contains no executable code and that it is not instantiated. See also the rule Interface class instantiated. INTERFACE_WITH_CODE
Write-only property found. A property is either missing a Get accessor or it is explicitly declared WriteOnly. Alternatively, the property is partially write-only in that the Get accessor has a smaller scope than the corresponding Set or Let. In the latter case the write-only and read/write status depends on scope. A write-only property is not logical. There is no security in letting one write a value but not read it back. Where write-only access is desired, it's better to use a method such as SetValue. This makes the design explicit. WRITEONLY
This group of rules handles procedures that respond to events and calls to an interface. In classic VB these procedures are determined by their names: Object_Event and Interface_Method. In VB.NET the procedures are defined via keywords Handles, AddHandler and Implements.
Event handler should be Private. Event handlers should be marked Private to prevent other modules from calling them directly. If it is necessary that the functionality be called from outside, the event handler should call the functionality in a separate procedure. In VB.NET, Private is also necessary to prevent child classes from overriding an event handler. Otherwise a less secure procedure could override a secure event handler, opening a potential security vulnerability. HANDLER_PRIVATE
Event handler called directly. Event handlers should not be explicitly called by other procedures. They should only execute as a response to an event. Calling handlers directly may lead to less manageable code. In addition, you may need to pass useless or fake arguments to the event handler. Create a new procedure, give it a meaningful name and move the called functionality to it. Should you later need to change the event handler, the changes will not accidentally affect the calling procedure. Tip: This problem may rarely be incorrectly shown for regular procedures that look like event handlers (say Pic1_Clack). In that case re-analyze the system including all COM files in the analysis. This will make event handler detection more accurate. Tip works with VB 4-6 and requires Project Analyzer Enterprise Edition. CALLED_DIRECTLY
Possibly orphaned event handler. A confusing Sub has been found. It looks like it may have originally been an event handler. It no longer handles any events, though. Check to see if the Sub should trigger as a response to some existing event or if it is obsolete. If it is a useful Sub, consider renaming it to show it is not an event handler. This review rule is heuristic and cannot tell for sure if a Sub actually was an event handler. In classic VB, this rule compares each Sub’s name against a list of well-known event names. In addition, it considers all Event signatures currently analyzed, including those found in COM files, to determine if a Sub looks like an event handler. In VB.NET this rule looks for Subs with event parameters (ByVal sender As System.Object, ByVal e As System.EventArgs). ORPHAN
Implementation should be Private. Procedures that implement an interface should be marked Private to prevent other modules from calling them directly. If it is necessary that the functionality be called from outside, the implementation and the functionality should be in separate procedures. In VB.NET, Private is also necessary to prevent child classes from overriding the implementation of an interface. Otherwise a less secure procedure could override a secure implementation, opening a potential security vulnerability. IMPL_PRIVATE
Implementation called directly. A procedure that implements an interface should not be explicitly called by other procedures. It should execute as a response to the interface it implements. A direct call may lead to less manageable code. Create a new procedure, give it a meaningful name and move the called functionality to it. Should you later need to change the interface, or the implementation, the changes will not accidentally affect the calling procedure. CALLED_DIRECTLY
Too many parameters. A procedure has a lot of parameters. The more parameters you have, the more difficult the procedure is to use. Consider dividing the procedure. Target at a maximum of 5 parameters. PARAMS
ByVal/ByRef missing. A parameter has no ByVal or ByRef declaration. The default in VB 3-6 is ByRef. In VB.NET, the default is ByVal.
Use of ByRef should be limited to where it is clearly required. ByVal is safer because it prevents values of passed parameters from being changed accidentally. Since bugs can easily occur with implicit ByRef, add either ByVal or ByRef to all parameters. Exception: explicit ByRef is unavailable in VB3.
If you migrate VB 3-6 code to VB.NET, there is yet another catch. The VB.NET upgrade wizard adds ByRef automatically. Add ByVal/ByRef manually to prevent this. NO_BYVAL
ByVal parameter written to. Procedure changes the value of a ByVal parameter. Procedures should not assign values to their formal parameters directly. Instead, procedures should declare a local variable for this purpose. Changing the value of a parameter may lead to errors that are hard to catch. When writing modifications to the procedure later, a developer may assume that the parameter retains its original value, even when it has changed. On the other hand, if a ByVal parameter is later changed to ByRef (due to optimization, for example), changes written to the parameter will propagate to the callers causing unexpected results. — Alternative use: Enable this rule to detect ByVal parameters that might be intended as 'out' parameters and should be changed to ByRef. See also: ByRef parameter returns a value. BYVAL_WRITTEN
ByRef parameter read only, consider ByVal. A ByRef parameter is not changed by the procedure. Consider passing the parameter by value (ByVal) instead of by reference (ByRef). This will prevent errors later. A future modification could accidentally change the value of the ByRef parameter, which could cause problems when the changed value is passed back to the callers. ByVal protects the callers from these unexpected changes. ByRef may sometimes be necessary for speed optimization, though. You can use this review rule to find ByRef parameters that could safely be made ByVal without changing the current logic. BYREF_READONLY
Pass ByRef. This rule is intended for manual review and does not necessarily indicate a problem. A parameter is being passed by reference in a procedure call. The value of the parameter may change during the call. Verify that a change is really expected by the caller. If the change is unexpected, it may cause an error in the caller or later in the program. — Use of this rule: Detect potentially unexpected changes to data. Review all call locations to find potential problems. This rule only reports ByRef passing where the value may change; if the value cannot change, this rule doesn't fire. PASS_BYREF
ByRef parameter returns a value. A procedure returns a value in a ByRef parameter. It is undesirable to return values via 'out' parameters. Callers of the procedure may not expect changes to the arguments. An unexpected change may result in an error that is hard to detect. Values should only be returned as function return values. Consider changing the parameter to ByVal and possibly returning the value in the function return value. If you need to return several values, consider a user-defined type as the return data type. If you really need to use an 'out' parameter, name it clearly to make it obvious. Out parameters in Functions and Properties are especially undesirable as they cause unexpected side effects. They are a bit less problematic for Subs, which do not have any other return value. For this reason, Project Analyzer offers the option 'Ignore for Subs'. Specifics: The value change does not necessarily happen in the procedure indicated. The parameter may be passed ByRef further to another procedure that does the writing. For an interface parameter the change may occur in the procedure(s) that implement the interface. If it's an Overridable procedure, writing may occur in Overrides methods in descendant classes. See also: ByVal parameter written to. BYREF_RETVAL
Rewrite Sub as Function. A Sub returns a value in an 'out' parameter. You should rewrite it as a Function for clarity. — A Sub with an 'out' parameter may be useful as an optimization. ByRef may avoid an expensive copy of the argument, whereas a function return value is always a copy. This optimization is related to String parameters and .NET Structures. SUB2FUNC
Optional parameter missing default value. Optional parameters should declare an explicit default value so that users will know what to expect when omitting the parameter. If you upgrade to VB.NET, the migration wizard will add a default value. The default for Variant parameters will be Nothing, not Missing, which can make your code behave differently. NO_DEFAULT
Optional parameter in exposed procedure. The Optional keyword presents a cross-language interoperability problem in VB.NET. Even though Optional parameters are a powerful and useful feature, they are not compatible with all .NET languages. The default value will be ignored and callers are required to pass all parameters. Therefore, Optional parameters should not be used with exposed procedures. For best interoperability, consider replacing Optional parameters with Overloads or several versions of the same procedure. You may also consider a ParamArray, even though this is not always the best choice. OPTIONAL_EXPOSED
Object parameter not cleared, Object variable not cleared, Object variable clearing violation. Array [parameter] not deallocated, Array deallocation violation. This group of rules monitors that the code explicitly clears all object variables and deallocates all dynamic arrays after use. This is useful for reducing the memory footprint of the program. See Explicit clearing of variables.
Resource not released, Resource releasing violation. These rules monitor that the code explicitly releases all Win API resource handles after use. This is useful for preventing resource leaks. See Resource leak detection.
Class design style rules form a group of issues related to class definitions, hierarchies and class members.
NameCheck. A name does not conform to the naming standards you have set with Project NameCheck. Change the name. NAMECHECK
Metric exceeds limits. A metric value exceeds the limit you have set. METRICS
The comment directive parameter for the rules on this page is STYLE.
Code review rules
©Aivosto Oy Project Analyzer Help