views:

279

answers:

8

I'm trying to generate prime numbers based on user input.

this is what i have so far but i just can't seem to figure it out:

Console.Write("Please enter the number of prime numbers you would like to see:");
        int numberOfPrimes = Convert.ToInt32(Console.ReadLine());

        for (int x = 0; x < numberOfPrimes; x++)
        {
            for (int a = 2; a <= x ; ++a)
            {
                bool prime = true;
                for (int b = 2; b < a; ++b)
                {
                    if (a % b == 0)
                    {
                        prime = false;
                    }//end if
                }//end double nested for
                if (prime == true)
                {
                    Console.WriteLine(a);
                }//end if
            }//end nested for
        }//end for
+3  A: 

You should be able to see why your results are wrong quite easily if you look at your loop structures. Step through them by hand (it won't take long).

The reason that you are getting your current results is that not every iteration of the outer loop (x < numberOfPrimes) produces a result - it will skip quite a few iterations due to the way the inner loop is structured.

What you really need to do is restructure the inner loops. Your innermost loop works fine, and should detect any prime numbers. Your second loop, however, should only test numbers that haven't yet been tested. Also, it should stop looping once you find a prime number.

a_m0d
i figured that i just cant figure out how to structure it.
Thatguy
I'm going to resist posting the answer because a_m0d is trying to hard to get you to do it yourself, but here's another hint: you need to get rid of the outer loop (you only want to count a up from 2 once)
Jimmy
@Jimmy - The outer loop is there so that he can generate enough numbers. There are other ways to do it as well, but this is his structure, and I'm just trying to help him get his structure working for him.
a_m0d
when i change it to a <= 100 it runs through the prime numbers up to 100 and prints them multiple times depending on what i input for numberOfPrimes. i think i need to remove the first for loop.
Thatguy
i got it working thanks.
Thatguy
@Thatguy - I don't think you need to remove it. You really have to step through all this in your head. Your innermost loop determines if a number is prime or not. How does it know what number to check? That is what the second loop is for. All you have to do is make sure that your second loop does not start over from 0 every time, and that it stops when you find a prime number.
a_m0d
@Thatguy - what did you change? I can edit that into my answer so that others looking for a solution can get the help that they need as well.
a_m0d
i took out the first loop, added the variable "count" changed the remaining outermost loop to count < numberOfPrimes, and added a incrementor to the if statement that displayed the number if it tested to be prime.
Thatguy
A: 

Once you do get ithe loops sorted out, you only have to check for b < sqrt(a), any higher and you would have found the other factor first.

Spike
Umm, he specifically tagged it as homework himself, so I would say it is homework.
a_m0d
Oh yeah. Opthamologist time again.
Spike
A: 

Can you tell me what purpose x has? Is it the counter of prime numbers? Is it the highest number to check for primeness? Can it be both?

Gabe
A: 

What you are looking for is called "Sieve of Eratosthenes." As I'm not into doing people's homework, this is the only clue I'm going to give you. The algorithm is easily found on the internet.

Muad'Dib
That's not really his problem - his algorithm could produce prime numbers with just a little restructuring. Okay, its not the most efficient algorithm ever, but it will work.
a_m0d
well, true, but if he knows what to search on he might find sample code
Muad'Dib
+1  A: 

You should structure your code better, it's really messy the way you do it now. Have a method IsPrime(int x) which returns true if x is prime and false otherwise.

Then, to generate numberOfPrimes primes, you can do something like this:

for ( int primeCount = 0, currentPrime = 2; primeCount < numberOfPrimes; ++currentPrime )
  if ( IsPrime(currentPrime) )
  {
    // do whatever you want with currentPrime, like print it
    ++primeCount;
  }

Or use the Sieve of Eratosthenes, which is a much faster method.

To figure out if a number x is prime or not, try all of its factors between 2 and Sqrt(x). Why only Sqrt(x)? Because if a*b = x, then x / b = a and x / a = b, So you would check everything twice, and also check things you shouldn't if you went up to x / 2 or even x.

So something like this if you want to use the IsPrime(x) function:

// i <= Sqrt(x) <=> i * i <= x
for ( int i = 2; i * i <= x; ++i )
  if ( x % i == 0 )
    return false;

return true;

But I suggest you use the sieve of Eratosthenes, as it's much faster. You can also optimize things so you don't check even numbers, since an even number is never prime, except for 2 (both in the sieve and the naive method). Treat x = 2 as an edge case and then start checking every other number (3, 5, 7, 9, 11 etc.)

IVlad
A number is also prime when it has no prime divisors; and it's much faster to check whether x is divisible by prime numbers less than sqrt(x) than it is to check whether x is divisible by any number less than sqrt(x). So you really want to pass a list of primes to your 'IsPrime' function...
Kirk Broadhurst
True, but you might as well use the sieve of Eratosthenes then. What you say is true and does have speed benefits, but it's only optimizing a naive solution that will lose to the sieve anyway, and using a list. Just use the sieve if you want the best algorithm.
IVlad
A: 

The next prime number (x) - is the number which cant be devided by all primes s, that s<=sqrt(x). So you can use function like

public bool CheckAndAddPrime(int number,List<int> primes)
{
    var sqrt = Math.Sqrt(number);
    foreach(var prime in primes)
    {
        if(prime>sqrt) break;
        if(number % prime == 0) return false;    
    }

    primes.Add(number);
    return true;
}

And than you can get primes like

var primes = new List<int>();
Enumerable.Range(2,int.MaxValue).Where(x => x.CheckAndAddPrime(x,primes)).Take(YouCountOfPrimes);
Steck
Also it is possible to add "2" as pre-defined prime and check only odd numbers
Steck
A: 
var primes = Enumerable.Range(1, numberOfPrimes )
    .Where(x => x != 1 &&
      !Enumerable.Range2, (int)Math.Sqrt(x)).Any(y => x != y && x % y == 0));

copied from codethinked.com

static void Main(string[] args)
{
    foreach (int no in get_first_k_primes(10))
    {
        Console.Write(" "+no.ToString() );
    }
}

public static List<int> get_first_k_primes(int k)
{
    var primes = new List<int>();

    primes.Add(2);

    int i  = 3;


    while(primes.Count < k)
    {
        if(is_prime(i))
            primes.Add(i);

        i += 2;
    }

    return primes;
}

public static bool is_prime(int n)
{
    if (n % 2 == 0 && n != 2) return false;

    int m = (int)Math.Ceiling(Math.Sqrt(n));

    for (int i = 3; i < m; i += 2)
    {
        if (n % i == 0) return false;
    }

    return true;
}
TheMachineCharmer
Looks good, but it's not going to be a good answer for homework, in terms of learning how to code the prime number algorithm.
Kirk Broadhurst
Doesn't this only list the primes <= `numberOfPrimes`? I think the OP wants to list `numberOfPrimes` primes. Either way, you should avoid the sqrt method, it's very slow.
IVlad
I don't think OP is asking 'Please paste a bunch of code here for me to copy'. He is asking 'here is my code, help me fix it'.
Kirk Broadhurst
I thought this code will help him to identify problems in his code. I m really sorry for this but I m bit busy and hurriedly pasted whatever was available.
TheMachineCharmer
A: 

1. Rename your variables.

Firstly, if this is homework you will get bad marks (if your teacher is worth his salt) because you have meaningless variable names (yes, even numberOfPrimes is wrong and should be named requiredNumberOfPrimes. When I see this variable I am asking myself 'Is this how many he wants, or how many he has found?').

Secondly, it will help you understand where you are going wrong. Variables should be logically named according to what they represent. If you can't explain what your variables represent (e.g. a & b) then you probably can't explain what you are doing with them.

2. Look at your loops.

for (int x = 0; x < numberOfPrimes; x++)

The structure of a for loop is (initialise; 'should I continue?'; 'each loop do this'). Therefore in your loop

  • You are continuing until x is equal to or great than numberOfPrimes*.
  • Each time you go through the loop you are adding 1 to x.

Are you sure this is what you want to do? x appears to represent the number of primes you have found. So why not increment it when you find a prime, rather than when you start a loop?

for (int a = 2; a <= x ; ++a)
for (int b = 2; b < a; ++b)

You are looking at each integer between 2 and x, inclusive. And for each of those integers a, you are looking at every integer between a and 2 inclusive. What are you going to do with these integers?

Every time you loop through your top-level loop (the x loop), you are going to start your a loop from scratch, and every time you loop through your a loop you will start your b loop from scratch.

So if x is 10, you run through a once (a=2), then you run through a again (a=2, a=3), then you run through a again (a=2, a=3, a=4), then...

3. Collect your results rather than writing them to Console.

var primes = new List<int>();

It's so easy. When you find a prime, primes.Add(a);. Then you know how many primes you have found (primes.Count), you can use the list of primes to efficiently determine the next prime, and you can use the list later on if required.

Kirk Broadhurst