Varför fastnar programmet i en loop? C++ (Nybörjare)

Permalänk
Medlem

Varför fastnar programmet i en loop? C++ (Nybörjare)

Hej!
Har för 1 vecka sedan satt igång med att försöka självlära mig programmering och har då gått igenom SoloLearns enkla kurs i C++ grunder.
Nu är det så att jag försöker öva på det jag lärt mig genom att koda ett första program som ska dela ut kort ur en kortlek men jag har råkat på ett problem jag inte förstår.
Av någon anledning fastnar programmet i en loop när man försöker printa ca 45+ kort ur kortleken samtidigt. Förklara gärna varför detta händer men se till att jag då också förstår förklaringen d.v.s. om ni använder komplicerade ord så vill jag gärna att det förklaras så en nybörjare i min nivå har lätt för att förstå.

Om ni vill påpeka hur jag kan struktura koden bättre eller ett bättre sätt till att koda detta programmet så skulle det gärna uppskattas.

#include <iostream> #include "Deck.h" #include "string" #include <cstdlib> deck::deck() :card{ "Ace", "King", "Queen", "Jack","Ten","9","8","7","6","5","4","3","2" }, suit{"Clubs","Diamonds","Spades","Hearts"}, card_short{ "A", "K", "Q", "J","T","9","8","7","6","5","4","3","2" }, suit_short{ "c","d","s","h" } { check = 0; } deck::~deck() { } void deck::deal() { cards = shuffle(); std::cout << cards; } std::string deck::shuffle() { failsafe = 0; //Är bara till för att förhindra en oändlig loop tills problemet är löst. c = rand() % 12; //Bestämmer vilket värde kortet har som printas. s = rand() & 3; //Bestämmer vilken färg kortet har som printas. while (dup_check(c, s)) { //Denna while loop säkerställer att vi inte printar en kopia på något kort men fastnar av någon anledning i en loop när vi försöker printa 40+ kort. c = rand() % 12; s = rand() & 3; } return card_short[c]+suit_short[s]+" - "+card[c]+" of "+suit[s]; } bool deck::dup_check(int x, int y) { if (failsafe > 52) { //Denna if funktion kontrollerar så ingen oändlig loop är igång men bara tillfälligt tills problemet är löst. std::cout << "----- ----- ----- dup_check() stuck in infinite loop detected.\n"; return false; } for (int z = 0; z < check; z++) { if (dupx[z] == x && dupy[z] == y) { //Av någon anledning när vi försöker printa 40+ kort så loopas denna if funktion hela tiden.. varför? failsafe++; //Berättar för programmet hur många gånger vi loopat utan att komma vidare (tillfällig lösning). return true; } } dupx[check] = x; dupy[check] = y; check++; return false; }

[Deck.cpp] Här är koden som fastnar i en loop när vi printar 40+ kort.

#pragma once class deck { public: deck(); ~deck(); std::string hand; std::string cards; std::string shuffle(); void deal(); bool dup_check(int x, int y); private: std::string const card[13]; std::string const suit[4]; std::string const card_short[13]; std::string const suit_short[4]; int c; int s; int check; int dupx[52]; int dupy[52]; int failsafe; };

[Deck.h] Tror inte denna behövs för felsökningen men inkluderar ändå

#include <iostream> #include <string> #include "Deck.h" #include <time.h> void deal(deck x) { int n; std::cout << "\nHow many cards do you want to deal?\n"; std::cin >> n; while (std::cin.fail()) { std::cin.clear(); std::cin.ignore(); std::cout << "No symbols or letters allowed. Try again.\n"; std::cin >> n; } if (n != 1) std::cout << "\nDealing your cards:\n"; else std::cout << "\nDealing your card:\n"; for (int z = 0; z < n; z++) { x.deal(); std::cout << std::endl; } std::cout << "\n\nDo you want to deal again?\n"; } int main() { srand(time(NULL)); std::string input; deck hand; deck *phand = &hand; std::cout << "Welcome to Card Generator 2017! Have fun generating cards.\n"; while (input != "quit"&&input != "n"&&input != "no") { deal(*phand); do { std::cin >> input; if (input != "yes"&&input != "quit"&&input != "y"&&input != "n"&&input != "no") std::cout << "'" << input << "'" << " is not a valid answer, please try again or type 'quit' to quit.\n"; } while (input != "yes"&&input != "quit"&&input !="y"&&input != "n"&&input != "no"); } }

[main.cpp]

PS: Är du en duktig programmerare som antingen har lust att lära mig programmering eller kan hjälpa mig få ihop en bra självstudieplan som t.ex. tar upp att nästa steg är att kolla på x video för att lära dig y och sedan försöker du koda z program med det du lärt dig så vill jag gärna att du tar kontakt med mig via pm.
Anledningen till att jag behöver självlära mig är det att jag tyvärr saknar möjligheten att gå en riktig programmeringsutbildning i min situation.

Visa signatur

i7 3930K - Noctua NH-D14 - MSI 1080 - Asus Sabertooth x79 - Corsair Vengeance DDR3 LP 4x4GB - Crucial MX100 256GB - AOC q2770Pqu 1440p - Corsair RM650x - Fractal Design R5 - Logitech G403 - Filco Majestouch-2 - Denon AVR-2113 - Dali Zensor 1

Permalänk
Medlem
Skrivet av Justin91:

c = rand() % 12; //Bestämmer vilket värde kortet har som printas.

Jag tror att du aldrig delar ut några tvåor. Heltalsresten vid division med tolv hamnar mellan noll och elva (inklusivt). Tvåorna finns på index 12 i din array.

Skrivet av Justin91:

s = rand() & 3; //Bestämmer vilken färg kortet har som printas.

Det här fungerar, men jag sätter en peng på att du inte förstår hur, för i så fall hade du nog använt modulo-operatorn här också.

Skrivet av Justin91:

bool deck::dup_check(int x, int y) {
if (failsafe > 52) { //Denna if funktion kontrollerar så ingen oändlig loop är igång men bara tillfälligt tills problemet är löst.
std::cout << "----- ----- ----- dup_check() stuck in infinite loop detected.\n";
return false;
}
for (int z = 0; z < check; z++) {
if (dupx[z] == x && dupy[z] == y) { //Av någon anledning när vi försöker printa 40+ kort så loopas denna if funktion hela tiden.. varför?
failsafe++; //Berättar för programmet hur många gånger vi loopat utan att komma vidare (tillfällig lösning).
return true;
}
}
dupx[check] = x;
dupy[check] = y;
check++;
return false;

Din algoritm har en 52 element lång array för valör (dupx) och en lika lång array för färg (dupy) som fylls på med redan utdelade kort i index check. När antalet redan utdelade kort ökar så ökar chansen att kortet man kollar redan är utdelat (metoden returnerar true).

Om du har delat ut 51 kort och ska dela ut det 52:a så kommer det att i genomsnitt ta 52 anrop av dup_check() innan den returnerar false (jag tar inte hänsyn till buggen med tvåorna här).

Din failsafe är alltså feltänkt, men jag tycker programmet borde fungera - om än ineffektivt.

Skrivet av Justin91:

Om ni vill påpeka hur jag kan struktura koden bättre eller ett bättre sätt till att koda detta programmet så skulle det gärna uppskattas.

En mer van programmerare hade haft en lista med kvarvarande kort för att slippa slumpningen där alla kort deltar i urvalet av nästa kort som ska delas ut. När man delar ut ett kort tar man bort det från listan.

Datastrukturer som listor, stackar, associativa arrayer och hashtabeller är bra att lära sig. Google eller en bra bok om datastrukturer och algoritmer är din vän.

Permalänk
Medlem
Skrivet av KAD:

Jag tror att du aldrig delar ut några tvåor. Heltalsresten vid division med tolv hamnar mellan noll och elva (inklusivt). Tvåorna finns på index 12 i din array.

Wow att jag aldrig upptäckte detta. Det är faktiskt sant och är en stor del i varför den fastnar i en loop. Anledningen till att jag gick ner till 12 respektive 3 i värdet på modulo operatorerna är det att det löste ett tillfälligt problem där programmet vägrade kompilera så jag trodde att det berodde på att jag råkade leta efter kort i index 13 respektive 4.

Skrivet av KAD:

Det här fungerar, men jag sätter en peng på att du inte förstår hur, för i så fall hade du nog använt modulo-operatorn här också.

Jo jag vet hur modulo operatorn fungerar och '&' symbolen råkar vara en missklick då jag tänkte ha modulo operatorn där också. Angående varför bitwise & operatorn fungerar är något jag inte förstår för tillfället men ska kolla upp det.

Skrivet av KAD:

Din algoritm har en 52 element lång array för valör (dupx) och en lika lång array för färg (dupy) som fylls på med redan utdelade kort i index check. När antalet redan utdelade kort ökar så ökar chansen att kortet man kollar redan är utdelat (metoden returnerar true).

Om du har delat ut 51 kort och ska dela ut det 52:a så kommer det att i genomsnitt ta 52 anrop av dup_check() innan den returnerar false (jag tar inte hänsyn till buggen med tvåorna här).

Din failsafe är alltså feltänkt, men jag tycker programmet borde fungera - om än ineffektivt.

Sant och detta förstår jag redan men då det är mitt första program så visste jag inte hur jag kunde lösa det på ett annat sätt.
Av någon anledning fortsätter programmet gå in i en oändlig loop som aldrig tar stopp när vi printar 50+ kort trots att jag fixat modulo random variablerna så tvåorna printas samt tagit bort failsafe.

Skrivet av KAD:

En mer van programmerare hade haft en lista med kvarvarande kort för att slippa slumpningen där alla kort deltar i urvalet av nästa kort som ska delas ut. När man delar ut ett kort tar man bort det från listan.

Datastrukturer som listor, stackar, associativa arrayer och hashtabeller är bra att lära sig. Google eller en bra bok om datastrukturer och algoritmer är din vän.

Wow du har helt rätt här. Men hur skapar man en lista som man kan slumpa ett kort ur och sedan ta bort från listan?
Ska googla på datastrukturerna du nämnde och läsa på.

Tack så mycket för hjälpen!

Visa signatur

i7 3930K - Noctua NH-D14 - MSI 1080 - Asus Sabertooth x79 - Corsair Vengeance DDR3 LP 4x4GB - Crucial MX100 256GB - AOC q2770Pqu 1440p - Corsair RM650x - Fractal Design R5 - Logitech G403 - Filco Majestouch-2 - Denon AVR-2113 - Dali Zensor 1

Permalänk
Medlem

Finns det någon nackdel med att använda "using namespace std;"?

Brukar själv använda det för att slippa skriva "std::" före var och varannan rad.

Visa signatur

[4770k delid] [1080 EK] [PG348Q] [Custom loop][1.75TB SSD]

Permalänk
Medlem

@theo0019: Anledningen till att jag skippade det var bara för att vänja mig vid att använda :: symbolen när det behövs samt förstå vad exakt som kräver std:: .

Visa signatur

i7 3930K - Noctua NH-D14 - MSI 1080 - Asus Sabertooth x79 - Corsair Vengeance DDR3 LP 4x4GB - Crucial MX100 256GB - AOC q2770Pqu 1440p - Corsair RM650x - Fractal Design R5 - Logitech G403 - Filco Majestouch-2 - Denon AVR-2113 - Dali Zensor 1

Permalänk
Medlem
Skrivet av Justin91:

Wow att jag aldrig upptäckte detta. Det är faktiskt sant och är en stor del i varför den fastnar i en loop. Anledningen till att jag gick ner till 12 respektive 3 i värdet på modulo operatorerna är det att det löste ett tillfälligt problem där programmet vägrade kompilera så jag trodde att det berodde på att jag råkade leta efter kort i index 13 respektive 4.
Jo jag vet hur modulo operatorn fungerar och '&' symbolen råkar vara en missklick då jag tänkte ha modulo operatorn där också. Angående varför bitwise & operatorn fungerar är något jag inte förstår för tillfället men ska kolla upp det.
Sant och detta förstår jag redan men då det är mitt första program så visste jag inte hur jag kunde lösa det på ett annat sätt.
Av någon anledning fortsätter programmet gå in i en oändlig loop som aldrig tar stopp när vi printar 50+ kort trots att jag fixat modulo random variablerna så tvåorna printas samt tagit bort failsafe.
Wow du har helt rätt här. Men hur skapar man en lista som man kan slumpa ett kort ur och sedan ta bort från listan?
Ska googla på datastrukturerna du nämnde och läsa på.

Tack så mycket för hjälpen!

Det finns i standard-biblioteket, åtminstone i <vector> eller en vector. Det är ingen linked list men den är en datastruktur som du kan krympa och nog den lättaste att jobba med som nybörjare, tycker jag. Annars kan du fylla en array med random kort och alltid plocka det sista kortet. Då har du en index-variabel som räknar nedåt varje gång du plockar ett kort från arrayen. Så slipper du trixa med olika datastrukturer. Om jag förstått problemet rätt dvs

Permalänk
Medlem
Skrivet av theo0019:

Finns det någon nackdel med att använda "using namespace std;"?

Brukar själv använda det för att slippa skriva "std::" före var och varannan rad.

Nackdelen är så vitt jag vet att om du har flera namespaces med liknande metoder, så kan det bli en konflikt. Fast i mindre projekt såhär där man bara leker runt så finns det ju förstås ingen nackdel.

Permalänk
Medlem

@Alotiat: Vad exakt finns i dessa bibliotek? Fylla en array med random kort och sedan en index variabel som räknar neråt tycker jag låter som en jättebra ide nu när man är en nybörjare.

Visa signatur

i7 3930K - Noctua NH-D14 - MSI 1080 - Asus Sabertooth x79 - Corsair Vengeance DDR3 LP 4x4GB - Crucial MX100 256GB - AOC q2770Pqu 1440p - Corsair RM650x - Fractal Design R5 - Logitech G403 - Filco Majestouch-2 - Denon AVR-2113 - Dali Zensor 1

Permalänk
Medlem
Skrivet av theo0019:

Finns det någon nackdel med att använda "using namespace std;"?

Brukar själv använda det för att slippa skriva "std::" före var och varannan rad.

Skrivet av Justin91:

@theo0019: Inte vad jag vet. Anledningen till att jag skippade det var bara för att vänja mig vid att använda :: symbolen när det behövs samt förstå vad exakt som kräver std:: .

Om man är ny eller gör småsaker: nej det spelar ingen roll. Fortsätt använda det om ni vill.

I större projekt undviker man helst att inkludera onödiga saker från ::std. Just för att man har flera namespaces som kan kollidera eller förvirra. Eller om man är erfaren och vet exakt vilka komponenter man vill använda från ::std.

@Justin91 Du kan läsa om vector här vector. Det är alltså en array som du kan förstora och förminska. Alltså en bättre version av en vanlig array med massa praktiska funktioner till. Men om du är nybörjare kanske det är lättast att vänta med det till senare tillfälle. Eller så läser du igenom och prövar implementera det

Permalänk
Medlem
Skrivet av Justin91:

Wow du har helt rätt här. Men hur skapar man en lista som man kan slumpa ett kort ur och sedan ta bort från listan?
Ska googla på datastrukturerna du nämnde och läsa på.

En enkel lösning är att bara ha en array med alla kort och en variabel som håller reda på storleken. Sen kör man bara följande algoritm:

Card deck[52]; int size = 52; do { int i = rand() % size; // Slumpa fram ett kort. std::swap(deck[i], deck[size - 1]); // Byt plats på det valda kortet och det "sista" kortet. --size; // Minska storleken med 1. } while (size > 0); // Upprepa tills alla kort slumpats.

Detta är ett exempel på en in-place algoritm, d.v.s. resultatet sparas i arrayen som används för algoritmen istället för att skapa en ny array.

Permalänk
Medlem

@Alotiat: Tack för förklaringen och länken.

@perost: Helt klart bästa enkla lösningen föreslagen hittills enligt mig då det är kort och simpelt.

Visa signatur

i7 3930K - Noctua NH-D14 - MSI 1080 - Asus Sabertooth x79 - Corsair Vengeance DDR3 LP 4x4GB - Crucial MX100 256GB - AOC q2770Pqu 1440p - Corsair RM650x - Fractal Design R5 - Logitech G403 - Filco Majestouch-2 - Denon AVR-2113 - Dali Zensor 1

Permalänk
Medlem
Skrivet av Alotiat:

Om man är ny eller gör småsaker: nej det spelar ingen roll. Fortsätt använda det om ni vill.

I större projekt undviker man helst att inkludera onödiga saker från ::std. Just för att man har flera namespaces som kan kollidera eller förvirra. Eller om man är erfaren och vet exakt vilka komponenter man vill använda från ::std.

@Justin91 Du kan läsa om vector här vector. Det är alltså en array som du kan förstora och förminska. Alltså en bättre version av en vanlig array med massa praktiska funktioner till. Men om du är nybörjare kanske det är lättast att vänta med det till senare tillfälle. Eller så läser du igenom och prövar implementera det

Jaha tack, då är det nog lika bra att börja vänja sig vid att använda std::.

Visa signatur

[4770k delid] [1080 EK] [PG348Q] [Custom loop][1.75TB SSD]