- Instruction contains multiple declarations - whenever you declare multiple variables on a single line, you're hindering readability and making it harder to maintain your code.
- Variable 'name' is implicitly Variant - @RubberDuck's answer @RubberDuck's answer pointed it out, but if you're not explicitly specifying a type for a variable, it implicitly gets declared as a
Variant
; it won't change how your code runs, but it makes it harder to tell who's supposed to be what... especially with names like that. - Member 'name' is implicitly Public - If an access modifier isn't specified, module members (procedures) are
Public
by default. That's potentially confusing, because in most other languages module members are private by default - hence, it's a potential maintainability issue; being explicit about access modifiers eliminates this problem. Also, procedures that are only used inside a module should bePrivate
. - Use of obsolete Call statement - VBA has quite a bit of history, and some keywords exist only to support legacy code written in older versions: the
Call
statement is one of such. If you need to call a method, just call that method - drop theCall
keyword and the parentheses, and you're done. - Variable 'name' is never assigned - some variables are never assigned a value; if they're referred to (good thing they're not), then you almost certainly have a bug. Otherwise, you have dead code.
- Variable 'name' is never used - some variables are never referred to; if they're also assigned (good thing they're not), then the assigned value is never used, and you have dead code.
- Instruction contains multiple declarations - whenever you declare multiple variables on a single line, you're hindering readability and making it harder to maintain your code.
- Variable 'name' is implicitly Variant - @RubberDuck's answer pointed it out, but if you're not explicitly specifying a type for a variable, it implicitly gets declared as a
Variant
; it won't change how your code runs, but it makes it harder to tell who's supposed to be what... especially with names like that. - Member 'name' is implicitly Public - If an access modifier isn't specified, module members (procedures) are
Public
by default. That's potentially confusing, because in most other languages module members are private by default - hence, it's a potential maintainability issue; being explicit about access modifiers eliminates this problem. Also, procedures that are only used inside a module should bePrivate
. - Use of obsolete Call statement - VBA has quite a bit of history, and some keywords exist only to support legacy code written in older versions: the
Call
statement is one of such. If you need to call a method, just call that method - drop theCall
keyword and the parentheses, and you're done. - Variable 'name' is never assigned - some variables are never assigned a value; if they're referred to (good thing they're not), then you almost certainly have a bug. Otherwise, you have dead code.
- Variable 'name' is never used - some variables are never referred to; if they're also assigned (good thing they're not), then the assigned value is never used, and you have dead code.
- Instruction contains multiple declarations - whenever you declare multiple variables on a single line, you're hindering readability and making it harder to maintain your code.
- Variable 'name' is implicitly Variant - @RubberDuck's answer pointed it out, but if you're not explicitly specifying a type for a variable, it implicitly gets declared as a
Variant
; it won't change how your code runs, but it makes it harder to tell who's supposed to be what... especially with names like that. - Member 'name' is implicitly Public - If an access modifier isn't specified, module members (procedures) are
Public
by default. That's potentially confusing, because in most other languages module members are private by default - hence, it's a potential maintainability issue; being explicit about access modifiers eliminates this problem. Also, procedures that are only used inside a module should bePrivate
. - Use of obsolete Call statement - VBA has quite a bit of history, and some keywords exist only to support legacy code written in older versions: the
Call
statement is one of such. If you need to call a method, just call that method - drop theCall
keyword and the parentheses, and you're done. - Variable 'name' is never assigned - some variables are never assigned a value; if they're referred to (good thing they're not), then you almost certainly have a bug. Otherwise, you have dead code.
- Variable 'name' is never used - some variables are never referred to; if they're also assigned (good thing they're not), then the assigned value is never used, and you have dead code.
Thanks for posting this wonderful code! I copied your code into a new VBA project, and ran Rubberduck code inspections. There were a few false positives (working on it), but if I remove them I'm still left with all these:
Suggestion: Instruction contains multiple declarations - VBAProject.Module1, line 182
Suggestion: Instruction contains multiple declarations - VBAProject.Module1, line 157
Suggestion: Instruction contains multiple declarations - VBAProject.Module1, line 4
Suggestion: Instruction contains multiple declarations - VBAProject.Module1, line 2
Suggestion: Member 'msg' is implicitly Public - VBAProject.Module1, line 180
Suggestion: Member 'Onoffuti' is implicitly Public - VBAProject.Module1, line 156
Suggestion: Member 'redu' is implicitly Public - VBAProject.Module1, line 130
Suggestion: Member 'aling' is implicitly Public - VBAProject.Module1, line 122
Suggestion: Member 'calcheck' is implicitly Public - VBAProject.Module1, line 95
Suggestion: Member 'cal1' is implicitly Public - VBAProject.Module1, line 86
Suggestion: Member 'cal' is implicitly Public - VBAProject.Module1, line 77
Suggestion: Member 'MainStart' is implicitly Public - VBAProject.Module1, line 6
Error: Variable 'strNameSurname3' is never assigned - VBAProject.Module1, line 135
Error: Variable 'strNameSurname4' is never assigned - VBAProject.Module1, line 136
Error: Variable 'strMessage' is never assigned - VBAProject.Module1, line 182
Error: Variable 'strMessage2' is never assigned - VBAProject.Module1, line 182
Error: Variable 'wd' is never assigned - VBAProject.Module1, line 4
Hint: Variable 'strNameSurname3' is never used - VBAProject.Module1, line 135
Hint: Variable 'strNameSurname4' is never used - VBAProject.Module1, line 136
Hint: Variable 'strMessage' is never used - VBAProject.Module1, line 182
Hint: Variable 'strMessage2' is never used - VBAProject.Module1, line 182
Hint: Variable 'wd' is never used - VBAProject.Module1, line 4
Warning: Use of obsolete Call statement - VBAProject.Module1, line 74
Warning: Use of obsolete Call statement - VBAProject.Module1, line 72
Warning: Use of obsolete Call statement - VBAProject.Module1, line 71
Warning: Use of obsolete Call statement - VBAProject.Module1, line 70
Warning: Use of obsolete Call statement - VBAProject.Module1, line 69
Warning: Use of obsolete Call statement - VBAProject.Module1, line 63
Warning: Use of obsolete Call statement - VBAProject.Module1, line 55
Suggestion: Variable 'strMessage' is implicitly Variant - VBAProject.Module1, line 182
Suggestion: Variable 'y' is implicitly Variant - VBAProject.Module1, line 182
Suggestion: Variable 'j3' is implicitly Variant - VBAProject.Module1, line 182
Suggestion: Variable 'q' is implicitly Variant - VBAProject.Module1, line 157
Suggestion: Variable 'd' is implicitly Variant - VBAProject.Module1, line 157
Suggestion: Variable 'p' is implicitly Variant - VBAProject.Module1, line 157
Suggestion: Variable 'ws' is implicitly Variant - VBAProject.Module1, line 4
Suggestion: Variable 'r3' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'i4' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'j2' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'j1' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'r1' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'i2' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'i1' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'i3' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'k' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'j' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'i' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'r' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'r2' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 'c' is implicitly Variant - VBAProject.Module1, line 2
Suggestion: Variable 's' is implicitly Variant - VBAProject.Module1, line 2
- Instruction contains multiple declarations - whenever you declare multiple variables on a single line, you're hindering readability and making it harder to maintain your code.
- Variable 'name' is implicitly Variant - @RubberDuck's answer pointed it out, but if you're not explicitly specifying a type for a variable, it implicitly gets declared as a
Variant
; it won't change how your code runs, but it makes it harder to tell who's supposed to be what... especially with names like that. - Member 'name' is implicitly Public - If an access modifier isn't specified, module members (procedures) are
Public
by default. That's potentially confusing, because in most other languages module members are private by default - hence, it's a potential maintainability issue; being explicit about access modifiers eliminates this problem. Also, procedures that are only used inside a module should bePrivate
. - Use of obsolete Call statement - VBA has quite a bit of history, and some keywords exist only to support legacy code written in older versions: the
Call
statement is one of such. If you need to call a method, just call that method - drop theCall
keyword and the parentheses, and you're done. - Variable 'name' is never assigned - some variables are never assigned a value; if they're referred to (good thing they're not), then you almost certainly have a bug. Otherwise, you have dead code.
- Variable 'name' is never used - some variables are never referred to; if they're also assigned (good thing they're not), then the assigned value is never used, and you have dead code.
The indentation is insufficient, which makes the code harder to read. A good rule of thumb would be that whenever you're writing code inside a code block (like between Sub...End Sub
or If...End If
), you should add an indentation level (tab). Indentation is also inconsistent - that's even worse, it completely defeats the whole purpose of indentation - the indent of If
(block begin) should should match the indent of End If
(block end).
Avoid extraneous indentation, too - there's no reason for the For
block to be 2 tabs right of the r1
assignment; both should actually be lined up:
r1 = Range("n1").SpecialCells(xlCellTypeLastCell).Row
For i2 = 2 To r1
It looks like the CalCheck
procedure (whatever that means) could be completely replaced with Excel formulae and conditional formattings - VBA isn't a silver bullet (I swear!), sometimes a good old formula works much better & faster!