Wyjątek "Integer division by zero" po wykonaniu pętli

// Liczby Pierwsze.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>

using namespace std;

int number, i, j, N;

int main()
{
	cout << "Liczba prob: ";
	cin >> N;
	for (j = 0; j < N; j++)
	{
		cout << "Podaj liczbe: ";
		cin >> number;
		for (int i = 2; i <= (number / 2) + 1 && number % i != 0; i++) cout << i << endl;
		if (number % i != 0) cout << "TAK" << endl;
		else cout << "NIE" << endl;
	}
    return 0;
}

Wyrzuca mi kod po wykonaniu pętli w linii if (number % i != 0) cout << "TAK" << endl; . A błąd jest taki:

Nieobsłużony wyjątek w lokalizacji 0x009B5ACA w Liczby Pierwsze.exe: 0xC0000094: Integer division by zero.

I się zastanawiam, bo "i" jest różne od zera. Nie wiem co jest źle.

Ale i masz wykorzystane tylko w tej linijce

for (int i = 2; i <= (number / 2) + 1 && number % i != 0; i++) cout << i << endl;

Później i nie ma już wartości bo wychodzisz z pętli więc masz 0. Musisz dać dalszą część kodu w nawiasach { } . Inaczej for wykonywany jest tylko dla
cout << i << endl;

Bo uzywasz zmiennych globalnych - tak w skrócie, a konkretniej, masz jedno i zadeklarowane globalnie, a drugie i lokalnie.

To jest dziwne:

for (int i = 2; i <= (number / 2) + 1 && number % i != 0; i++) cout << i << endl;

W przypadku liczby (na przykład) number = 1 cały ten for nawet się nie wykona.

Po drugie:
i jest jak najbardziej równe 0, dlatego że zadeklarowałeś i jako zmienną globalną: int number, i, j, N;
Musisz pamiętać, że linijka

for (int i = 2; i <= (number / 2) + 1 && number % i != 0; i++) cout << i << endl;

posiada swoją lokalną zmienną i, która umiera po wykonaniu się przytoczonej pętli for(), a potem znów masz globalną i równą właśnie 0.

Rozwiązanie: Zadeklaruj zmienną i tam gdzie rzeczywiście jej potrzebujesz i zadbaj o nadanie jej prawidłowej wartości - nie zostawiaj jej niezainicjalizowanej!

Cały ten program jest zły, tak się nie pisze, nie Deklaruj zmiennych globalnych, a jak w ogóle Deklarujesz coś to zmienną, to Nadaj jej początkową wartość. A tę logikę sprawdzania najlepiej zamknąć w funkcji.

Ulepszyłem trochę program:

// Liczby Pierwsze.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>

using namespace std;

bool prime(int n)
{
	int i;
	for (i = 2; i < (n / 2) && n % i != 0; i++);
	if ((n % i != 0) && (n != 1) || (n == 2)) return true;
	else return false;
}

int main()
{
	int number, N;
	cout << "Liczba prob: ";
	cin >> N;
	for (int j = 0; j < N; j++)
	{
		cout << "Podaj liczbe: ";
		cin >> number;
		prime(number);
		if (prime(number)) cout << "TAK" << endl;
		else cout << "NIE" << endl;
	}
    return 0;
}

Obliczenia wykonuje poprawnie, ale czy program jest napisany dobrze stylowo?

1. Ten fragment:

        prime(number);
        if (prime(number)) cout << "TAK" << endl;

Dlaczego powtarzasz wywołanie prime() dwukrotnie i dopiero za drugim razem sprawdzasz rezultat? Zrób to tylko raz.

2. Wciąż pozostawiasz zmienne bez wartości początkowej: int number, N;
Nadawaj im wartość od razu: int number = 0, N = 0;

3. Nazewnictwo zmiennych! Zmienna number nic nie mówi, bo czym ten numer rzeczywiście jest? Zmienna N już tym bardziej utrudnia zrozumienie.
edit: Dam przykład: Zmienna N przechowuje liczbę prób, wnioskując po komunikacie dla użytkownika linijkę wyżej. Więc niech zmienna ta nazywać się będzie triesNumber <- liczba prób.
Od razu czytelniej, prawda?

4. Moim zdaniem Za dużo rzeczy starasz się zmieścić w jednej linijce. To trochę utrudnia odnalezienie się w kodzie.

Kmizek napisał(a):

Ulepszyłem trochę program:

// Liczby Pierwsze.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>

using namespace std;

bool prime(int n)
{
	int i;
	for (i = 2; i < (n / 2) && n % i != 0; i++);
	if ((n % i != 0) && (n != 1) || (n == 2)) return true;
	else return false;
}

int main()
{
	int number, N;
	cout << "Liczba prob: ";
	cin >> N;
	for (int j = 0; j < N; j++)
	{
		cout << "Podaj liczbe: ";
		cin >> number;
		prime(number);
		if (prime(number)) cout << "TAK" << endl;
		else cout << "NIE" << endl;
	}
    return 0;
}

Obliczenia wykonuje poprawnie, ale czy program jest napisany dobrze stylowo?

W software design nie ma jednej drogi, można coś zrobić dobrze na kilka sposobów. Języki mają swoje zasady, jak na przykład PEP w Pythonie, co może być pomocne w designie. Ten kod bym jeszcze uprościł, bo za bardzo trzeba zwolnic analizując tą funkcję (Jakbyś miał 100 takich w projekcie? Trzeba przyspieszać:), poza tym jest deklaracja zmiennej bez przypisania wartości, czego sie nie robi ):

bool isPrime(int n){
	for (int i = 2; i < n / 2 + 1; ++i){
		if (n % i == 0)
			return false;
		}
	return true;
}

Od razu widać, że jest to (bardzo nieoptymalne) sprawdzenie czy liczba jest pierwsza.

    if ((n % i != 0) && (n != 1) || (n == 2)) return true;
    else return false;

Te dwie linijki mógłbyś zmienić na takie:

//na początku funkcji:
if (n == 1) return false;
if (n == 2) return true;
//...
//pętla

return (n % i != 0);

Jeszcze się dopytam. Czemu jest taki wymóg, że zawsze trzeba przypisać wartość początkową zmiennej przy jej deklaracji?