Skip to main content
4 of 4
added 5 characters in body
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 name Range is hiding Global.Range, which may or may not be a problem (it isn't, in this case). Hiding/shadowing existing declarations in wider scopes is generally a bad idea.
  • 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 embarrassing 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 source If cell.Value = searchCell.Value Then BetterSearch = "Match" Exit Function 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