Program nie dziala poprawnie

Question

Program ma za zadanie losowac liczby z zakresu 1-9 i zadna liczba nie moze sie powtorzyc. Co tutaj jeszcze dodac lub zmienic ? Dzięki :)

#include <iostream>
#include <conio.h>
#include <cstdlib>
#include <ctime>
using namespace std;
int main()
{
    const int n=3,m=3;
    int tab[n][m],i,j,a,b,k,x,los;
    srand(time(0));
    for(i=0;i<n;i++)
    {
        for(j=0;j<m;j++)
        {
        if(j==0)
        {
            tab[i][j]=rand()%10;
            
        }
    
        else
            {
                do{
                los=rand()%10;
                b=0;
                
                for(k=0;k<j;k++)
                {
                    if(tab[i][k]==los)
                        b++;
                }    
                }while(b);    
                tab[i][j]=los;
            
            }    x++;    
        }
    }
    for(i=0;i<n;i++)
    {
        for(j=0;j<m;j++)
        {
            cout << tab[i][j] <<" ";
        }
        cout << endl;
    }
    getch();
    return 0;

 

Comments

Wywaliłes pytanie i zadałeś od nowa? Napisałem dosyć długi komentarz, drugi raz mi sie nie chce :P

---

Znalazłem :D

odpowiedź @RafalS z nie wiadomo po co ukrytego pytania:

}    x++;

inkrementacja niezainicjalizowanej zmiennej, co jest bez sensu, ale na szczescie nigdzie ta zmienna nie jest wykorzystywana, wiec lepiej zeby jej nie było.

Dramatyczne nazwy zmiennych: a,b,x. Nazwij je tak, żeby było wiadomo za co odpowiadają, tak jak to zrobiłeś ze zmienną los. Jeśli już totalnie nie wiesz jak ma się dana zmienna nazywać bo będzie używana tylko do tymczasowego zapamietania jakiejś wartości to nazwij ją tmp. Nie jest to najlepsze rozwiązanie ale przynajmniej mówi programiście, żeby nie rozkminiał po co jest ta zmienna.

int tab[n][m], i, j, a, b, k, x, los;
srand(time(0));
for (i = 0; i < n; i++)

nie musisz i nie powinieneś inicjować zmiennych iteracyjnych petli for poza nia, zrób tak:

for (int i = 0; i < n; i++)

To do while(b) może się zaciać w nieskończoność. Sprawdzaj tam czy dlugosc tablicy nie jest wieksza od przedzialu. Jeśli spróbujesz wylosować 15 róznych liczb z przedziału 0-10 to program się zatnie.

Dodatkowo to co zrobiłeś tu:

do {
	los = rand() % 10;
	b = 0;

	for (k = 0; k < j; k++)
	{
		if (tab[i][k] == los)
			b++;
	}
} while (b);

Jest troszkę nadmiarowe. Gdy będziemy mieć tablicę 10000 na 10000000 i np tab[0][0] nam wylosuje 0 to jeśli dla drugiego elementu wylosuje też zero to niepotrzebnie przeiterujesz po całej tablicy. Zrób tam break gdy znajdziesz.

Dodatkowo 

if (tab[i][k] == los)

tutaj porównujesz śmieci (niezaincjalizowane wartości) to znaczy, że może się tak trafić, że akurat w tych śmieciach będzie jakas wartość z przedziału w którym losujesz :P

Answer 1

Po pierwsze program wydaje się działać jak najbardziej poprawnie bo warunek:

                do{
                los=rand()%10;
                b=0;
                 
                for(k=0;k<j;k++)
                {
                    if(tab[i][k]==los)
                        b++;
                }    
                }while(b);    
                tab[i][j]=los;
             

sprawdza czy  wierszu i nie ma liczby los (idziesz po wierszu za pomocą pętli for) - możesz to łatwo sprawdzić dając np. n = 7, m = 9 i zauważysz, że w wierszach liczby się nie powtarzają, w kolumnach tak, jak liczba ma być unikalna w całej tablicy to musisz sprawdzić wszystkie wiersze a nie tylko i-ty

Po drugie tak bardzo na około napisanego programu to dawno nie widziałem, ale rada jest taka, że wszędzie tam gdzie elementy mają być unikatowe sprawdzanie za każdym razem czy dany element nie wystąpił jest po prostu słabe - są to niepotrzebne obliczenia, zdecydowanie lepszym pomysłem jest np. tablica booli, gdzie jeżeli pod i-tym indeksem jest 0 to dana liczba jeszcze nie została użyta, a jeżeli jest 1 to znaczy że była używana i należy losować jeszcze raz