Skip to main content
1 of 4
Mathieu Guindon
  • 71.5k
  • 8
  • 113
  • 250

I'm getting cross-eyed, what mistake am I making?

Several.

  • Parameter Range is declared as a String, but clearly used as if it were a Range object. It should be declared As Range.
  • The function is implicitly Public. Better if explicitly so.
  • Parameters are implicitly passed ByRef, but there's no justification for it; they should be passed ByVal.
  • Parameter searchCell is an implicit Variant, but it's used as if it were a Range object; declare it As Range.
  • Function returns an implicit Variant, but really returns a String. Signature should specify As String for the return type.
  • Local variable cell isn't declared, which means it's an on-the-fly implicit Variant. Declare it explicitly, As Range.
  • Code that compiles with undeclared variables doesn't have Option Explicit specified, which means VBA will happily compile and run any typo. Avoid stupid problems, specify Option Explicit at the top of every single module, and declare every single variable.
  • "No Match" return value is needlessly re-assigned on every iteration.
  • Function name is camelCase, but public members in every single VBA type library are consistently PascalCase.
  • Indentation isn't consistent.

Rubberduck (an open-source VBE add-in project I manage) would have picked up most of these points with its static code analysis.

Option Explicit Public Function BetterSearch(ByVal searchCell As Range, ByVal source As Range) As String Dim cell As Range For Each cell In Range If cell.Value = searchCell.Value Then BetterSearch = "Match" Exit For End If Next BetterSearch = "No match" End Function 

IMO the function would be much more useful if it returned a Boolean rather than a "magic string". True when found, False when not found.

Mathieu Guindon
  • 71.5k
  • 8
  • 113
  • 250