whats wrong with this function

Thread view:

Enable EMail Alerts

Start New Thread

Thread rating:

KISSnACDC - 27 Jul 2007 13:12 GMT

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

Reply to this Message

Baz - 27 Jul 2007 13:44 GMT

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

> I can't figure it out, I'm trying to create a function to find prime numbers.
>
[quoted text clipped - 25 lines]
>
> End Function

Reply to this Message

KISSnACDC - 27 Jul 2007 13:50 GMT

I'm sorry, what?

> One obvious problem is that you never assign a value to the variable Val,
> therefore it's always zero.
[quoted text clipped - 29 lines]
> >
> > End Function

Reply to this Message

Baz - 27 Jul 2007 16:02 GMT

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

> I'm sorry, what?
>
[quoted text clipped - 31 lines]
> > >
> > > End Function

Reply to this Message

Douglas J. Steele - 27 Jul 2007 14:00 GMT

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

Signature

Doug Steele, Microsoft Access MVP
http://I.Am/DougSteele
(no e-mails, please!)

>I can't figure it out, I'm trying to create a function to find prime
>numbers.
[quoted text clipped - 26 lines]
>
> End Function

Reply to this Message

KISSnACDC - 27 Jul 2007 16:00 GMT

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'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.
[quoted text clipped - 63 lines]
> >
> > End Function

Reply to this Message

Douglas J. Steele - 27 Jul 2007 16:34 GMT

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.

Signature

Doug Steele, Microsoft Access MVP
http://I.Am/DougSteele
(no e-mails, please!)

> Oky doky, I see what I did wrong, thanks!
>
[quoted text clipped - 76 lines]
>> >
>> > End Function

Reply to this Message

KISSnACDC - 27 Jul 2007 17:18 GMT

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.

> Create a loop that passes values to the function, and stop the loop once
> you've hit 100 primes:
[quoted text clipped - 83 lines]
> >> >
> >> > End Function

Reply to this Message

John W. Vinson - 27 Jul 2007 16:36 GMT

>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]

Reply to this Message

Douglas J. Steele - 27 Jul 2007 17:25 GMT

> 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.

Signature

Doug Steele, Microsoft Access MVP
http://I.Am/DougSteele
(no e-mails, please!)

Reply to this Message