Code review rules: Problem list
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 accross 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.
Default control naming. A control has not been named with a descriptive name. VB has generated a default name instead, like Text1, List2 etc. This problem is available for VB 3-6. DEFAULT_NAMING
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. You can set the limit yourself, and Project Analyzer reports names that are equal to or below the limit. SHORT_NAME
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
Fixed file number found. Using fixed file numbers 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
Case Else branch missing. There are more Select statements than Case Else branches in a procedure. One or more Select block is missing the default Case Else condition. You can require each Select to have a default branch in case an unexpected value comes up. If you want to detect unexpected values while debugging but let your final executable run without interruption, put a Debug.Assert False in each Case Else that may execute due to an unexpected input value. CASE_ELSE
Microsoft.VisualBasic imported. Use objects and methods in the .NET Framework and stay clear of those defined in the Microsoft.VisualBasic namespace, which Microsoft provides only for backward compatibility. The native .NET methods are often faster and offer more options.
Multiple statements on one 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
Return value discarded. The return value is ignored at a call to a function. Although it is not necessary to actually use a function's return value, simply ignoring it could indicate a problem in the caller's logic. Review the call to determine if the return value can be ignored safely. This rule also verifies Declare Sub statements that could be rewritten as Declare Function if DLL analysis is enabled. This problem does not apply to VB3 where return values must be used at all times. See also the issue Dead return value. RETVAL
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/Gosub statement found. Use of Goto/Gosub is bad programming practice and should be avoided when possible. Jumping around with Goto easily leads to unstructured control flow structure and spaghetti code. Gosub has low performance and isn't supported by VB.NET. GOTO, GOSUB
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. - Aivosto VB Friend is a tool that splits If..Then statements to multiple lines. SINGLE_LINE_IF
Call statement found. The Call statement is no longer needed. You may remove it. CALL
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
Exit/Return statement found. Use of the Exit and Return statements may indicate unstructured program flow, much the same way as the Goto statement. An Exit or a Return statement causes an immediate jump out of a loop or a procedure. It makes the loop/procedure have several exit points. Loops and procedures should have only one exit point at the end. It is especially important to avoid jumping out of With..End With blocks because the With expression may not be released properly if End With is not executed. — There is a case where VB requires a premature Exit Sub/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 (introduced in VB 2005). Project Analyzer allows these uses.
Note: The Return statement is equally avoidable in all versions of Visual Basic, even though its functions are different. In VB 3-6, it is used to return from a GoSub jump. In VB.NET, it is used to set the function return value and quit. EXIT
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
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
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
This is a set of rules on the declaration of data types and scopes.
Function with type character. A function is declared ending in one of the old-style $#%!&@. Instead of MyFunction$( ), you should use MyFunction( ) As String, etc. TYPE_CHAR
Variable/parameter/constant with type character. A variable, procedure parameter or constant is declared ending in one of the old-style $#%!&@. Instead of Year%, you should use Year As Integer, etc. TYPE_CHAR
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
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
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.
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
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
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. FIXEDLEN
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
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 ...'. This rule allows you to require explicit variable initialization in VB.NET. Since fields of Structures cannot have initializers, the rule ignores them. NO_INITIALIZER
Option Explicit missing. A module is missing its Option Explicit statement. You should always use Option Explicit. Using it makes VB require declaration of all variables. This way you avoid using unnecessary implicit Variants. 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
Option Strict missing. A file does not define Option Strict On. In .NET, Option Strict On enforces type-safe code by allowing only widening data type conversions. It also disables late binding. MISSING_STRICT
Object variable clearing violation, Array deallocation violation, Object variable/parameter not cleared, Array/Array parameter not deallocated, Resource not released, Resource releasing violation. This is a group of rules, under the Style category, for developers who want to ensure that their code explicitly clears object variables, dynamic arrays and Win API resource handles after use. CLEAR
Explicit clearing of
variables
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.
Problem detection
Code review rules: Problem list
©Aivosto Oy - Project Analyzer Help Contents