0

I'm trying to basically do a VLOOKUP, but the content of my cell is too long for VLOOKUP to process. I therefore use this VBA script to search through a defined range:

Function betterSearch(searchCell, Range As String) For Each cell In Range If cell.Value = searchCell.Value Then betterSearch = "Match" Exit For End If betterSearch = "No match" Next End Function 

The function is called as (for example): =betterSearch(B33;'Master'!C:C)

However, I can't get a single output. I'm getting cross-eyed, what mistake am I making?

2
  • 3
    Shouldn't Range as String be Range as Range? Commented Aug 17, 2017 at 18:51
  • 2
    You variables should not have the same name as pre-existing methods. betterSearch(searchCell, Source As Range) would be better. Commented Aug 17, 2017 at 18:59

1 Answer 1

1

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.

Sign up to request clarification or add additional context in comments.

6 Comments

Thank you so much. I understand most of your comments, however, I run into the following oddity. If the Source Range is on the same worksheet as the searchCell, it works, but if the range is on a different worksheet, it returns a #VALUE! error?
Gah, typo / copy+paste error... should be For Each cell In source - fixed.
That one I caught ;) But it just wont work across worksheets?
hmm, it should work AFAICT. @Tampert replace Exit For with Exit Function - works on my machine =)
Works! The range it was searching through had a few #VALUE! errors of its own... Thanks again!
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.