0

This is the function that is giving the issue :

Its supposed to check if a string only has numbers 0-9

Public Function onlyNumbers(str As String)
        For i = 1 To Len(str)
        If Not (IsNumber(Mid(str, i, 1))) Then
            onlyNumbers = False
        End If
    Next
    onlyNumbers = True
End Function

Module:

    Dim value as string
For j = 2 to 2205
    value = Cells(j, 2)

    value = Trim(Replace(Replace(value, "-", ""), ".", ""))
   'Error gives on the if check (it highlights "value") :        
    If onlyNumbers(value) Then

   ' code goes on... no syntax error, execution only
Improve this question
10
  • 1
    Either add a call before or remove the (). Commented May 23, 2016 at 20:53
  • 1
    IsNumber is not vba. Try IsNumeric() Commented May 23, 2016 at 20:53
  • Also your UDF will always return true. You need to exit the function upon finding a false. Commented May 23, 2016 at 20:53
  • @ScottCraner in excel 2007 at least, that's how you return, setting a return gives syntax error Commented May 23, 2016 at 20:58
  • 1
    And the problem is you are using value a word that is already used in vba, change the variable to some non vba word like vlu or something. Commented May 23, 2016 at 21:11

6 Answers 6

Reset to default
3

Just to save all the bother - you don't even need a function or a loop for this, just use the Like operator:

Dim myString As String

myString = "12345"

If myString Like Application.Rept("[0-9]", Len(myString) Then
    MsgBox "myString is numbers only"
Else
    MsgBox "myString contains other characters that aren't 0-9"
End If

If you really want that to be a function then:

Function IsNumbersOnly(str As String) As Boolean
    IsNumbersOnly = str Like Application.Rept("[0-9]", Len(str))
End Function
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

Add:

 Dim value As String

at the beginning of the Sub.

Improve this answer

1 Comment

My bad, it had that line just got cut
1

It would appear people have given great advice in the comments, below is a working solution that takes in them and reduces the codebase.

Public Function onlyNumbers(ByVal str As String) As Boolean
For i = 1 To Len(str)
    If Not (IsNumeric(Mid(str, i, 1))) Then Exit Function
Next
onlyNumbers = True
End Function
Improve this answer

6 Comments

Actually, because you have already taken out the minus and decimal point. You don't need the function at all If IsNumeric(value) Then instead of If onlyNumbers(value) Then.
exiting function automatically asign its return value to false?
@Mojimi yes, unless you specify a boolean value, its default is false
Put something in this that talks about the problem with the use of Value as a variable, so this then is the correct answer.
@Mojimi Also, just incase it is unknown. You do not have to close an If statement with End If is it is running a single commend when triggered. I.e. if the content of the If state is a simple one line operation, you can put it on the one line and not add the End If as I have done. I have to do this a lot as code optimisation is a big task for me, however, it can make for difficult to follow code so I wouldn't say to do it if you're not sure or if the code is becoming overly complicated.
|
0

Cells() returns a range object.

You need to set value to the value of the range. Try Cells().value when calling your function.

value = Cells(j, 2)
value = Trim(Replace(Replace(value, "-", ""), ".", ""))
'Error gives on the if check :        
If onlyNumbers(value.value) Then
Improve this answer

3 Comments

Actually, unless you use the Set keyword before the assignment the Cells() method default property is .Value so that is what gets returned when you don't specify.
Nope, that wasn't it
That's neat -- I didn't realize that. Thanks Macro Man!
0

4 errors

1) Exiting the loop for correct logic as pointed by @ScottCraner

2) Variable name "value" already used by vba, likely conflicting(error from question title) as pointed by @ScottCrane

3) Next i missing

4) IsNumber is not in VBA, the correct is IsNumeric as pointed by @ScottCraner 

Public Function onlyNumbers(str As String)
        For i = 1 To Len(str)
        If Not (IsNumeric(Mid(str, i, 1))) Then
            Exit Function
        End If
    Next i
    onlyNumbers = True
End Function

Thanks for everyone who helped

Improve this answer

1 Comment

I would still suggest you don't need the function at all, you are looping character by character to look for non-numeric entries in the string, You can pass the whole string into IsNumeric and get the same result, but you would have the benefit of no extra function to go into and no loop to go around.
-1

I believe you are having a problem with your Cells call. Please change it to Cells(2, 10). This will be j2 as this API is Cells(row, column) by index

Dim Value As String
Value = Cells(2, 10)

Value = Trim(Replace(Replace(Value, "-", ""), ".", ""))
Debug.Print onlyNumbers(Value)

And your other piece had some incorrect syntax because you shoudl use IsNumeric

Public Function onlyNumbers(str As String)
    For i = 1 To Len(str)
    If Not IsNumeric(Mid(str, i, 1)) Then
        onlyNumbers = False
    End If
Next
onlyNumbers = True
End Function
Improve this answer

2 Comments

It's completely fine to use j in the Cells() method as long as j is a number, and your function will always return true
The original post did not have the variable J defined. So when I re-built the code, my assumption (correct or not) was that J was intended to be the column heading. The fact that the post was re-edited multiple times makes it appear as though I did not read - thanks for the negative from whomever that was. Missing from original post: Dim value as string For j = 2 to 2205

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.