1

I wrote a function that generates a "score" (1,0,-1) based on a input cell, which should contain a number. However, sometimes the input field might not be numeric and then the function should return the output "0".

Function ScoreRoE(RoE_Field As Range, goodval As Range, badval As Range)

    Dim RoE As Double, result As Double

    RoE = RoE_Field.Value

        If IsNumeric(RoE_Field.Value) = False Then
            result = "0"
        Else:
            If RoE >= goodval.Value Then
                result = "1"
            ElseIf RoE <= badval.Value Then
                result = "-1"
            Else:
                result = "0"
            End If
        End If

    ScoreRoE = result

End Function

When the input cell is numeric the function works correctly. However, when not it doesn't and just returns an error "#VALUE!"

Thanks a lot in advance!

Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
yannk
  • 51
  • 6
  • 3
    do your isnumeric test before you assign ROE its initial value. Otherwise you could be assigning a string to a double and things do not look promising there. – Forward Ed Jun 21 '19 at 17:01

2 Answers2

4

Declare RoE as Variant:

Function ScoreRoE(RoE_Field As Range, goodval As Range, badval As Range)

    Dim RoE As Variant, result As Double

    RoE = RoE_Field.Value

        If Not IsNumeric(RoE) Then
            result = 0
        Else
            If RoE >= goodval.Value Then
                result = 1
            ElseIf RoE <= badval.Value Then
                result = -1
            Else
                result = 0
            End If
        End If

    ScoreRoE = result

End Function

you cannot assign a text value to a double.


Personally I do not see the need for any of the variables:

Function ScoreRoE(RoE_Field As Range, goodval As Range, badval As Range)

    If Not IsNumeric(RoE_Field.Value) Then
        ScoreRoE = 0
    Else
        If RoE_Field.Value >= goodval.Value Then
            ScoreRoE = 1
        ElseIf RoE_Field.Value <= badval.Value Then
            ScoreRoE = -1
        Else
            ScoreRoE = 0
        End If
    End If

End Function
Scott Craner
  • 148,073
  • 10
  • 49
  • 81
  • @ForwardEd `¯\_(ツ)_/¯` – Scott Craner Jun 21 '19 at 17:05
  • Is there particular reason for using RoE instead of dealing with RoE_Field.Value directly aside from keystrokes? – Forward Ed Jun 21 '19 at 17:05
  • @ForwardEd I personally do not see the need to use any of the variables but wanted to keep as closely as possible to what the OP is using. – Scott Craner Jun 21 '19 at 17:06
  • not that it is REQUIRED, but shouldn't the function be assigned a variable type instead of letting it default to variant? – Forward Ed Jun 21 '19 at 17:09
  • @ForwardEd I usually leave it as variant when outputting to a sheet, but that is personal preference. – Scott Craner Jun 21 '19 at 17:10
  • @ForwardEd arguably if `RoE_Field.Value` (or any of the inputs) is an `Error` value, the function should return `CVErr(xlErrValue)` as a `Variant/Error`, not `0`. That can't be cleanly done without returning a `Variant`. – Mathieu Guindon Jun 21 '19 at 17:14
  • No, all those variables are not required. That was a great help, much leaner code this way – yannk Jun 21 '19 at 17:20
  • 1
    @yannk IF you want leaner, define `ScoreROE=0` at the get go. follow by a check to see if `RoE_Field.Value` IS a number. And then if it is do your two checks for good value then bad value. Everything else will be zero as that was what you initialized at the start. – Forward Ed Jun 21 '19 at 17:23
2

This is an interpretation of the intentions of the code:

Function ScoreRoe(roeInput As String, goodVal As String, badVal As String) As String

    If Not IsNumeric(roeInput) Or Not IsNumeric(goodVal) Or Not IsNumeric(badVal) Then
        ScoreRoe = "0"
    Else
        Dim goodValNumeric As Double: goodValNumeric = goodVal
        Dim badValNumeric As Double: badValNumeric = badVal
        Dim roeInputNumeric As Double: roeInputNumeric = roeInput

        If roeInputNumeric >= goodValNumeric Then
            ScoreRoe = "1"
        ElseIf roeInputNumeric <= badValNumeric Then
            ScoreRoe = "-1"
        Else
            ScoreRoe = "0"
        End If
    End If

End Function

The main idea is that the input is taken as String and then converted to Double for the comparison. Then if the input is not numeric, the returned value is 0 and if it is we go through If-Else.

In general, it could be a good idea to avoid : in VBA If-Else, in order to avoid unwanted problems like this one - VBA - How the colon `:` works in VBA code with condition

Vityata
  • 42,633
  • 8
  • 55
  • 100
  • 1
    Interesting take on the situation. Not the route I would naturally thought of, but nice way of coming at the issue from a different angle. – Forward Ed Jun 21 '19 at 17:13
  • @ForwardEd - well, I started by removing the `_` from the variables' names, then removed the `:`, then decided to do it completely different way :) – Vityata Jun 21 '19 at 17:15