Losowanie bez powtórzeń - nie działa funkcja sprawdzająca powtórzenia

0

Cześć wszystkim ;) nie byłem pewien czy wrzucić to tutaj czy do C++, ale tak właściwie to duuużo bliżej mi do newbie niż do programisty, więc piszę tu.

Mój problem dotyczy programu, który wczytuje 3 liczby do tablicy a później losuje dwie z nich bez powtórzeń.
Wygląda to tak:

 #include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

int Losuj( int tab[] );

bool Sprawdz( int tab[], int pula, int liczba );

/***********************************/

int Losuj( int tab[] )
{
   
   
    int i =( rand() % 3 );
    int liczba = tab[ i ];
    return liczba;
   
   
   
}

/***********************************/

bool Sprawdz( int tab[], int pula, int liczba )
{
    if( pula < 0 )
         return false;
   
    int i = 0;
    do
    {
        if( tab[ i ] == liczba )
        {
            return true;
            i++;
        }
    } while( i < pula );
   
    return false;
}

/***********************************/

int main()
{
    srand( time( NULL ) );
    int losy[ 3 ];
    int i = 0;
    do
    {
        cout << "Podaj liczbe: ";
        cin >> losy[ i ];
        i++;
    } while( i < 3 );
   
    i = 0;
    int proba = 0;
    int Los;
   
    do
    {
        if( Sprawdz( losy, i, Los ) == false )
        {
            Los = Losuj( losy );
           
            cout << Los << endl;
            proba++
            ;
        }
       
    } while( proba < 2 );
   
    return 0;
   
}

Wszystko działa - poza funkcją Sprawdz:

bool Sprawdz( int tab[], int pula, int liczba )
{
    if( pula < 0 )
         return false;
   
    int i = 0;
    do
    {
        if( tab[ i ] == liczba )
        {
            return true;
            i++;
        }
    } while( i < pula );
   
    return false;
}

, która albo nie działa w ogóle albo zawiesza program po wylosowaniu pierwszej liczby. Po jej zakomentowaniu program działa prawidłowo.

Dzięki za pomoc ;)

2
if( tab[ i ] == liczba )
{
    return true;
    i++;
}

i++ nigdy się nie wykonuje. Wyrzuć poza if.

4
        if( tab[ i ] == liczba )
        {
            return true;
            i++;
        }

Nigdy nie inkrementujesz i (jedyne miejsce gdzie to robisz jest nieosiągalne, bo jest zaraz za instrukcją return)

Powinno być:

        if( tab[ i ] == liczba )
        {
            return true;
        }
        i++;

Przy okazji: losowanie bez powtórzeń najłatwiej osiągnąć wypełniając kontener (np. tablicę) unikalnymi wartościami, a potem mieszając jej elementy. Wtedy możesz wziąć n pierwszych elementów i masz gwarancję braku powtórzeń.

I przy okazji, skoro jesteśmy już przy C++, unikaj używania rand: http://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful

0

Tak, tak, wybaczcie, to starsza wersja kodu gdy ta inkrementacja była jeszcze w warunku ;) Zauważyłem to wcześniej, ale wciąż ten sam efekt.

Możesz rozwinąć o co chodzi z "unikalnymi wartościami" i ich mieszaniem?

1
4mnysi4 napisał(a):

Możesz rozwinąć o co chodzi z "unikalnymi wartościami" i ich mieszaniem?

Na przykładzie totolotka. Masz wylosować 6 zwycięskich liczb z zakresu 1-49, bez powtórzeń. Najłatwiej (co wcale nie musi oznaczać najwydajniej pamięciowo na przykład) będzie zrobić kontener ze wszystkimi tymi wartościami:
[ 1, 2, 3, 4, ... 48, 49]
Wymieszać elementy, aby były ułożone losowo:
[39, 24, 1, 44, 14 ... 8, 2]
I w tym momencie w trywialny sposób możesz wybrać n losowych liczb z tego zakresu bez powtórzeń po prostu biorąc kolejne wartości z wymieszanego kontenera.

1
int Los;
do
{
    if( Sprawdz( losy, i, Los ) == false )
    {
        Los = Losuj( losy );
 
        cout << Los << endl;
        proba++;
    }
 
} while( proba < 2 );
  1. Sprawdzasz czy Los jest w tablicy. Najprawdopodobniej nie, bo to jest jakaś losowa wartość.
  2. Skoro nie ma to dajesz mu jakąś wartość z tablicy.
  3. Od następnej iteracji nigdy nie zwiększasz proba, bo if zawsze będzie fałszywy.

Edit: chyba sprawa jest jeszcze ciut bardziej zagmatwana :D
Sprawdz(losy, i, Los) - i ma wartość 0, więc w funckcji Sprawdz de facto sprawdzasz czy Los jest równy losy[0], więc jeśli Los == losy[1] lub Los == losy[2] to Sprawdz zwróci false, przez co powyższa pętla ma szansę się skończyć.

0

Poprawiłem nieco kod - teraz wygląda to tak:

 do
    {
        int Los = Losuj(losy);
        if( Sprawdz( losy, i, Los ) == false )
        {

            cout << Los << endl;
            proba++;
            int Los = 0;

teraz program się nie wiesza, ale wciąż zdarza mu się losować te samy liczby, teraz nawet częściej (pewnie przez to że teraz program się nie wiesza ;) ).

1
do
    {
        int Los = Losuj(losy);
        if( Sprawdz( losy, i, Los ) == false )
        {
 
            cout << Los << endl;
            proba++;
            int Los = 0; // definiujesz nową zmienną o nazwie Los, a nie przypisujesz do obecnie istniejącej. powinno być:
            Los = 0; // o tak
1
4mnysi4 napisał(a):

teraz program się nie wiesza, ale wciąż zdarza mu się losować te samy liczby, teraz nawet częściej (pewnie przez to że teraz program się nie wiesza ;) ).

if( Sprawdz( losy, i, Los ) == false )

i czyli pula ma wartość 0. Więc wylosujesz tylko pierwszą wartość tablicy (inne są odrzucane jako nieistniejące w tablicy), albo aż próby się skończą.

Uprzedzając pytanie: jeśli zrobisz

if (Sprawdz(losy, 3, Los) == false)

to pętla się nie skończy, bo zawsze za pierwszą próbą dostaniesz coś z tablicy, więc proba zawsze będzie przy wartości 1. Musisz mieć drugi warunek zakończenia pętli typu:

bool znaleziono = Sprawdz(losy, 3, Los);
if (znaleziono == false)
...

} while (znaleziono == false && proba < 2)
0

Właśnie to zauważyłem ;) myślę tylko jak tam "wcisnąć" inkrementację, żeby ta zmienna się zwiększała.

Edit:
Zrobiłem to (chyba) tak jak piszesz, teraz main() wygląda tak:

 int main()
{
    srand( time( NULL ) );
    int losy[ 3 ];
    int i = 0;
    int Los;
    bool znalazl=Sprawdz(losy, 3, Los);
    do
    {
        cout << "Podaj liczbe: ";
        cin >> losy[ i ];
        i++;
    } while( i < 3 );
 
    int proba = 0;
 
 
    do
    {
 
        Los = Losuj(losy);
 
        if (znalazl==false)
        {
 
            cout << Los << endl;
            proba++;
 
        }
 
    } while(znalazl==false &&  proba < 2 );
 
    return 0;
 
}

, ale liczby wciąż się powtarzają. W dodatku jeśli teraz podam 3 te same wartości, to i tak wylosuje z tego liczby.

0

Po kolei, bo się pogubimy. Ta pętla w moim wyobrażeniu miała tak wyglądać:

bool znalezione;
do
{
    Los = Losuj( losy );
    cout << "Wylosowana liczba: " << Los << endl;
    znalezione = Sprawdz(losy, 3, Los);        // pula tutaj == 3
    if(znalezione == false )
    {
        cout << "Wylosowana liczba nie w puli" << endl;
        proba++;
    }
 
} while(znalezione == false && proba < 2 );
  1. Zauważ, że pula tutaj jest równa 3. Czyli losujesz 1 liczbę z tablicy, przez co potem Sprawdz zawsze zwróci true, bo sprawdza czy dana liczba należy do tablicy.
  2. Jeśli dajesz mniejszą pulę jako argument funkcji Sprawdz, to po prostu sprawdzasz mniejszy fragment tablicy. Czyli tak jak na początku miałeś, dałeś pulę 0, przez co Sprawdz sprawdza czy wylosowana liczba jest równa pierwszemu elementowi tablicy losy[0].
  3. Nie do końca wiem, co chciałeś osiągnąć przy pomocy puli i prób, wg opisu to chciałeś wylosować 2 liczby z tej tablicy, czyli można najprościej tak:
wylosuj pierwszą liczbę
wylosuj drugą liczbę
    póki druga jest równa pierwszej
        powtórz losowanie drugiej liczby
0

Okej, od początku:

  1. Wpisuję do tablicy 3 różne liczby.
  2. Program losuje liczbę z tej tablicy.
  3. Sprawdza, czy ta liczba już wcześniej nie została wylosowana.
  4. Jeśli została - losuje inną.
  5. Jeśli nie została - wypisuje liczbę na ekran i losuje kolejną.
  6. Sprawdza czy padły już 2 liczby; jeśli nie - wraca do pkt 2.

Proba to zmienna która zwiększa się gdy wylosowana liczba zostanie wypisana, innymi słowy - gdy próba się powiedzie.
(jak teraz patrzę na kod to sam nie wiem po co dałem tam tą pulę, chyba tylko po to żeby mieć jakiś argument do zamknięcia pętli)

1

Czemu nie chcesz zrobić to po ludzku? Wtedy oto cały program:

#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;
 
int main()
  {
   int losy[3];
   for(int i=0;i<3;cin>>losy[i++]) cout<<"Podaj liczbe: ";
   srand(time(0));
   int pos=rand()%3;
   for(int i=0;i<3;++i) if(i!=pos) cout<<losy[i]<<endl;
   return 0;
  }

http://ideone.com/aJIX29

1

No to widać, że użycie funkcji Sprawdz jest błędne, bo sprawdza istnienie liczby w tablicy, do której wpisałeś wszystkie początkowe liczby. A powinieneś sprawdzić w tablicy, gdzie są wylosowane do tej pory liczby. Ale tych liczb nigdzie nie zapisujesz.
Skoro potrzebujesz tylko 2 liczb, to możesz zrobić tak jak w moim pseudokodzie, czyli:

  1. wylosować pierwszą
  2. wylosować drugą i sprawdzić czy ona jest równa pierwszej, jak tak to powtórz
    Żadnego sprawdzania w tablicy nie trzeba, wystarczy 1 zmienna do przechowywanie pierwszej wylosowanej liczby.
0

Wygląda na to, że sobie poradziłem ;)

 #include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

int Losuj( int tab[] );


/***********************************/

int Losuj( int tab[] )
{


    int i =( rand() % 3 );
    int liczba = tab[ i ];
    i++;
    return liczba;



}

/***********************************/


int main()
{
    srand( time( NULL ) );
    int losy[ 3 ];
    int i = 0;
    int Los;
    int liczba;
    do
    {
        cout << "Podaj liczbe: ";
        cin >> losy[ i ];
        i++;
    } while( i < 3 );

    int proba = 0;

    do
    {
        Los=Losuj(losy);
        if (Los!=liczba)
        {
            cout << Los << endl;
            liczba=Los;
            proba++;
        }


    }while(proba < 2);



    return 0;

}

Teraz liczby się losują bez żadnych powtórzeń. Dzięki wielkie za pomoc ;)

2
int liczba;   // liczba ma losową wartość

Jeśli kiedyś jakimś cudem liczba będzie miała wartość jak pierwsza wylosowana liczba, to program odrzuci pierwsze losowanie. Nawet tego nie odczujesz oczywiście, ale dobrą praktyką jest inicjalizowanie zmiennych przed użyciem. W tym przypadku należy:

int liczba = -200000000;  // nadać jakąś wartość która na pewno nie występuje w losowaniu
int Losuj(int tab[])
{
    int i = (rand() % 3);
    int liczba = tab[ i ];
    i++;                        // bezużyteczna instrukcja
    return liczba;
}

i++ praktycznie nic nie robi, bo i jest zmienna lokalną, która zaraz zniknie po wyjściu z funkcji. Poza tym tę funkcję można skrócić do:

int i = rand() % 3;
return tab[i];

i dalej do:

return tab[rand()%3];

1 użytkowników online, w tym zalogowanych: 0, gości: 1