8

This program is for printing prime numbers till the input given and separating every prime number with a comma.

void main(){

    int N, counter=0, isPrime;

    int k, j;

    cout << "Enter maximum range: ";

    cin >> N;

    for (j=2; j<=N; j++){

        isPrime = 0;
        k = 2;

        while (k<j){

            if (j%k==0){

                isPrime++;
            }
            k++;
        }
        if (isPrime==0){

            if (k==N){
                cout << j;
            }
            else{
                cout << j << ",";
            }
            counter++;
        }
    }
    cout << endl;
    system("pause");
}

It is only removing the last comma for prime number inputs, not for any other input. How can I fix this?

Input: 23
Output: 2,3,5,7,11,13,17,19,23
Input: 8
Output: 2,3,5,7,
Input: 9
Output: 2,3,5,7,
Zoe
  • 27,060
  • 21
  • 118
  • 148
Talha Gillani
  • 355
  • 1
  • 5
  • 9

6 Answers6

32

There is no need to if then else so much:

std::string delim = "";
for( auto&& item : vec )
{
   std::cout << delim << item;
   delim = ",";
}

No checking is needed for all cases, like the vector is empty or not.

If you accept an extra space in the beginning, just replace the string to char, and then the performance will be improved even more.

Carlo Wood
  • 5,648
  • 2
  • 35
  • 47
5

Don't remove the last comma. Instead insert commas before each entry except the first.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Ben
  • 34,935
  • 6
  • 74
  • 113
2

To easily remove the last comma you can use the '\b' character.

for(auto item : vec)
    std::cout << item << ", " ;
std::cout << "\b\b " << std::endl;
Community
  • 1
  • 1
Erman
  • 1,543
  • 17
  • 26
1

Just decide from a pre condition:

bool first = true;
for(j=2;j<=N;j++){
   // ...
   if(k==N) {
   if(!first) {
       cout << ',';
   }
   else {
       first = false;
   }
   cout<<j;
}
πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
0

The easiest way is to output the first or last value manually:

#include <iostream>
#include <cmath>

using namespace std;

int main() {
    int N, counter = 0;
    bool isPrime;

    cout << "Enter maximum range: ";
    cin >> N;
    if (N>=2) {
        cout << "2";
    }
    for (int j = 3; j <= N; ++j) {
        isPrime = true;
        for (int k = 2; k < sqrt(j)+1; ++k) {
            if (j % k == 0) {
                isPrime = false;
                break;
            }
        }
        if (isPrime) {
            cout << ", " << j;
            counter++;
        }
    }
    cout << endl;
    system("pause");
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
EvgeniyZh
  • 898
  • 9
  • 21
0

With minimal changes to your existing code:

#include <iostream>
#include <string>

using namespace std;

void main()
{
    int N, counter = 0, isPrime;
    string separator = ""; // none at first
    int k, j;

    cout << "Enter maximum range: ";

    cin >> N;

    for(j = 2; j <= N; j++)
    {

        isPrime = 0;
        k = 2;

        while(k<j)
        {

            if(j%k == 0)
            {

                isPrime++;

                break; // exit while loop
            }
            k++;
        }
        if(isPrime == 0)
        {
            // if(k==N) not needed
            cout << separator << j;
            separator = ","; // comma after first
            counter++;
        }
    }
    cout << endl;
    system("pause");
}

Explanation
Basically, I added a separator string which is blank at the start, i.e. the empty string, but is set to a comma for each output after the first. As such the cout statement will not print a comma before the first number, but will do so for each subsequent number being printed.

SirKM
  • 60
  • 5
  • You might want to explain what you've changed in the original code - this would make your answer more useful to others. Thanks. – rwp Apr 28 '18 at 16:27
  • I added comments to each line I changed. I thought it was clear. – SirKM Apr 29 '18 at 12:26