Strings should not be set to empty string
vbNullString is a special constant that denotes a null string (0), while "" is a literal empty string. For most purposes, the two are equivalent, but vbNullString is faster to assign and process, and takes less memory. vbNullString
is therefore preferred, however some non-VB APIs or components may not properly handle null strings, and their use should be tested.
Variable types should be specified
Declaring a variable without specifying its data type leaves the compiler to assign the type that seems the most appropriate - whether it’s what you need or not. Therefore you should always specify the data type.
Loop invariants should not be calculated inside the loop
Loop invariants are expressions whose values do not change during the execution of a loop. Placing the calculation of an invariant inside a loop is confusing, and inefficient because the resulting value will always be the same, yet it must be re-calculated each time through the loop. Therefore invariants should be calculated and stored before loop execution begins.
Static procedures should not be used
The local variables in a Static Sub or Function
are preserved between calls (meaning that these variables have the same lifetime as the owning module of their procedure). Static procedures should be avoided because they are difficult to test, and can result in unexpected behavior.
Expressions should not be too complex
The difficulty of understanding an expression increases for each of the And, Or and Xor
operators it contains.
A single expression’s complexity should not become too high to keep the code readable.
Return type should be specified for functions
Explicitly declaring a function’s return data type makes it easier to use the function correctly.
Error handlers should not declare their own error handlers
It is not possible to handle errors that arise inside error-handling code. Therefore, the declaration of a new error handler within an error-handling section will lead to unpredictable behavior, since the second error-handling routine is activated only after the first error handler has exited.
Forms should be unloaded and removed from memory
Because Visual Basic 6 does not automatically unload Form s after a program terminates, it is possible for them to remain in memory. Even after Unload ing, they may stay in memory if references remain. Therefore Form s should be explicitly Unload ed, and their references set to Nothing
.
Variable data types should be declared explicitly
Variables declared without the explicit specification of a data type are Variants
. Variants can be inefficient to use because at each interaction they are converted to the appropriate type for that interaction. Variants may be required for COM interactions, but even then their type should be specified explicitly.
$ should not be used in string variable names
Appending ’() versus Left().
Instead, Strings should be explicitly declared with the String datatype.
Select Case statements should have at least 3 Case clauses
`Select Case statements are useful when there are many different cases depending on the value of the same expression.
For just one or two cases however, the code will be more readable with If` statements.
The & operator should be used to concatenate strings
Both the ”+” operator and the ”&” operator can be used to concatenate strings. However, there is a complicated set of rules around the actual behavior of the ”+” operator, and whether you will get a concatenation operation, addition, a compiler error, or a Type mismatch
error. On the other hand, the ”&” operator can only perform concatenation, and is therefore preferred.
String-specific functions should be used
Variant functions are inefficient by nature, and should not be used with strings when string-specific versions, denoted by a `$ at the end of the name, are available.
This rule flags instances of these functions:
Left(), Mid(), Right(), Chr(), ChrW(), UCase(), LCase(), LTrim(), RTrim(), Trim(), Space(), String(), Format(), Hex(), Oct(), Str(), Error`
Strings should not be compared with empty string
It is far faster to compare the length of a string to 0 than it is to compare the string itself to empty string. Further, the LenB implementation is faster than the Len
implementation, and is therefore preferred.
GoTo statements should not be used
GoTo is an unstructured control flow statement. It makes code less readable and maintainable. Structured control flow statements such as If, For, While, or Exit
should be used instead.
Multiple variables should not be declared on the same line
Declaring multiple variable on one line is difficult to read and potentially misleading since the As
typing only applies to the last variable on the line.
Asc[W] should not be called on string constants
Because the value returned never changes, it is inefficient to call Asc/AscW
on a String constant. Use the numeric value instead.
Case clauses should not have too many lines
The Select statement should be used only to clearly define some new branches in the control flow. As soon as a Case clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of Case
clause should be extracted in a dedicated sub or function.
Chr[W]$() should not be used for certain characters
It is more efficient to skip the function invocation and use the predefined string constants rather than calling `Chr() for the following numbers:
0 - vbNullChar
8 - vbBack
9 - vbTab
10 - vbLf
11 - vbVerticalTab
12 - vbFormFeed
13 - vbCr
13+10 - vbCrLf | vbNewLine
34 - ""`
Type should be specified for parameters
Declaring a parameter without specifying its data type leaves the compiler to assign the type that seems the most appropriate - whether it’s what you need or not. Therefore you should always specify the data type.
Empty statements should be removed
Empty statements, i.e. `:, are usually introduced by mistake, for example because:
It was meant to be replaced by an actual statement, but this was forgotten.
There was a typo which resulted in the colon being doubled, i.e. ::`.
The bang (!) operator should not be used
Use of the bang (!) operator leads to late binding and results in inefficient code. Use the slightly more verbose dot (.
) notation instead.
Return of boolean expressions should not be wrapped into an if-then-else statement
Return of boolean literal statements wrapped into If-Then-Else
ones should be simplified.
Select statements should end with a Case Else clause
The requirement for a final Case Else clause is defensive programming. The clause should either take appropriate action or contain a suitable comment as to why no action is taken. Even when the Select covers all current values of an enum, a Case Else case should still be used because there is no guarantee that the enum
won’t be extended.
Option Base should not be used
The use of Option Base
to change the lower bound of an array’s index values results in confusing code.
Select Case statements should not be nested
Nested `Select Case structures are difficult to understand because you can easily confuse the cases of an inner Select Case as belonging to an outer statement. Therefore nested Select Case statements should be avoided.
Specifically, you should structure your code to avoid the need for nested Select Case statements, but if you cannot, then consider moving the inner Select Case` to another function.
Statements should be on separate lines
Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.
Unresolved directive in <stdin> - include::{noncompliant}[]
Write one statement per line to improve readability.
Unresolved directive in <stdin> - include::{compliant}[]
Magic numbers should not be used
Magic numbers make the code more complex to understand as it requires the reader to have knowledge about the global context to understand the number itself. Their usage may seem obvious when writing the code, but it may not be the case for another developer or later once the context faded away. -1, 0, and 1 are not considered magic numbers.
Boolean literals should not be redundant
A boolean literal can be represented in two different ways: {true} or {false}. They can be combined with logical operators ({ops}) to produce logical expressions that represent truth values. However, comparing a boolean literal to a variable or expression that evaluates to a boolean value is unnecessary and can make the code harder to read and understand. The more complex a boolean expression is, the harder it will be for developers to understand its meaning and expected behavior, and it will favour the introduction of new bugs.
Mergeable if statements should be combined
Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.
Merging if statements when possible will decrease the nesting of the code and improve its readability.
Methods should not be empty
An empty {operationName} is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty {operationName}s bring no functionality and are misleading to others as they might think the {operationName} implementation fulfills a specific and identified requirement.
There are several reasons for a {operationName} not to have a body:
It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
It is not yet, or never will be, supported. In this case an exception should be thrown.
The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.
Unused private variables should be removed
If a private field is declared but not used locally, its limited visibility makes it dead code.
This is either a sign that some logic is missing or that the code should be cleaned.
Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand and preventing bugs from being introduced.
Comments should not be located at the end of lines of code
Sub and function names should comply with a naming convention
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all function names match a provided regular expression.
Using hardcoded IP addresses is security-sensitive
Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:
Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:
The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).
Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.
Track uses of TODO tags
Source code should be indented consistently
Consistent indentation is a simple and effective way to improve the code’s readability. It reduces the differences that are committed to source control systems, making code reviews easier.
This rule raises an issue when the indentation does not match the configured value. Only the first line of a badly indented section is reported.
This rule verifies that single-line comments are not located at the ends of lines of code. The main idea behind this rule is that in order to be really readable, trailing comments would have to be properly written and formatted (correct alignment, no interference with the visual structure of the code, not too long to be visible) but most often, automatic code formatters would not handle this correctly: the code would end up less readable. Comments are far better placed on the previous empty line of code, where they will always be visible and properly formatted.