whats wrong with this function

G

Guest

I can't figure it out, I'm trying to create a function to find prime numbers.


Option Compare Database
Option Explicit

Public Function IsPrime(rngVal As Integer) As Variant
Dim ValPrime As Boolean
Dim x As Long
Dim ValGCD As Long
Dim Val As Long

If Val = 1 Or Val < 0 Or Val <> Int(Val) Or IsEmpty(rngVal) = True Then
IsPrime = "#VALUE"
Exit Function
End If
ValPrime = True

For x = 2 To Val - 1
If Val / x = Int(Val / x) Then
ValPrime = False
IsPrime = ValPrime
Exit Function
End If

Next x

IsPrime = ValPrime

End Function
 
B

Baz

One obvious problem is that you never assign a value to the variable Val,
therefore it's always zero.
 
D

Douglas J. Steele

You're passing rngVal to the function, but using Val inside it. Since Val is
never assigned a value, it's always going to be equal to 0.

Now, since rngVal is defined as an integer, there's no need to check rngVal
<> Int(rngVal). However, be aware that passing any non-integer value will
result in what's passed to the function being rounded. Passing 3.2 will
result in the function assuming a value of 3, passing 3.5 will result in the
function assuming a value of 4. As well, rngVal can never be Empty.

You've declared x as Long. Since rngVal is Integer, there's really no need
for x to be a Long Integer. Perhaps you want to change the declaration of
rngVal to Long as well. However, since rngVal and x are both integer values,
rngVal/x will always be an integer. You need to convert at least one of them
to a non-integer value in order to have the division done properly. It's
probably easiest just to make X a Single.

Try:

Public Function IsPrime(rngVal As Long) As Variant
Dim x As Single

If rngVal = 1 Or rngVal < 0 Then
IsPrime = "#VALUE"
Exit Function
End If

For x = 2 To (rngVal - 1)
If rngVal / x = Int(rngVal / x) Then
IsPrime = False
Exit Function
End If
Next x

IsPrime = True

End Function
 
G

Guest

Oky doky, I see what I did wrong, thanks!

Now if I wanted to only print the first 100 prime numbers would I have to
create a whole new function or could I do some minimal modification to this
function?
 
B

Baz

I can't say it any plainer. I think you need to learn a bit about VBA
before going any further.
 
D

Douglas J. Steele

Create a loop that passes values to the function, and stop the loop once
you've hit 100 primes:

I would have shown you the code, but I suspect that this is a homework
assignment. Given the fact that you obviously haven't learned VBA, I'd be
doing you a grave disservice if I just gave you the answer.
 
J

John W. Vinson

Oky doky, I see what I did wrong, thanks!

Now if I wanted to only print the first 100 prime numbers would I have to
create a whole new function or could I do some minimal modification to this
function?


You could just put in an outer loop. What do you want to do with the prime
numbers when you find them, though? put them in a Table? Print them out?

One other thing to make the program more efficient: you're testing a LOT more
divisors than are needed. There's no reason to divide by numbers that you know
aren't prime (if the number is divisible by 16 then you've already caught it
twice, dividing by 4 and by 2); and there's no reason to divide by a number
larger than the square root of the potential prime (if it has a larger factor
than the square root then it also has a smaller factor). Try changing the
inner loop:

For x = 2, 3 To int(Sqrt(rngval)) STEP 2
If rngVal / x = Int(rngVal / x) Then
IsPrime = False
Exit Function
End If
Next x

John W. Vinson [MVP]
 
G

Guest

It isn't a homework assignment, but I understand why you don't want to write
the code. I just started learning vba and have been having some trouble, but
thanks for all your help.
 
D

Douglas J. Steele

John W. Vinson said:
For x = 2, 3 To int(Sqrt(rngval)) STEP 2
If rngVal / x = Int(rngVal / x) Then
IsPrime = False
Exit Function
End If
Next x


Umm, what's that supposed to be, John? A new version of the For loop?

I suspect you meant

If rngVal Mod 2 = 0 Then
IsPrime = False
Exit Function
End If
For x = 3 To int(Sqrt(rngval)) STEP 2
If rngVal / x = Int(rngVal / x) Then
IsPrime = False
Exit Function
End If
Next x

Yeah, I meant to include that efficiency.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top