4 Gewinnt
-
Moin,
ich wollte mal fragen, ob jemand Lust hätte über ein kleines vier gewinnt von mir drüber zu schauen. Bisher habe ich mir über unregelmäßige Zeitabstände immer mal wieder C angeeignet unter anderem mit dem Buch "C von A bis Z". Jetzt wüsste ich gern, ob ich die bisherigen Angelegenheiten begriffen habe oder nicht. Es ist nämlich eine Sache, ob ein Programm funktioniert oder ob es sicher funktioniert.
#include <stdio.h> #include <string.h> #include <stdlib.h> #include <iso646.h> #include <time.h> #define MAX_BUFFER 255 void get_input( char buffer[], unsigned int amount ) { memset( buffer, 0, (amount==0) ? MAX_BUFFER : amount ); int tmp_c = 0; unsigned int counter = 0; while( (tmp_c=getchar()) != EOF and tmp_c != '\n' ) { if( counter != amount ) { buffer[counter] = tmp_c; counter++; } } buffer[counter] = '\0'; } void print_field( char game_field[][7] ) { int i,j; for( i=6; i>=0; i-- ) { printf( "\t" ); for( j=0; j<7; j++ ) { printf( "[%c]", (game_field[i][j]==2) ? ' ' : ((game_field[i][j]==0) ? 'X' : 'O') ); } printf( "\n" ); } } void ask_players( char players[][16] ) { printf( "Player 1 choose your name (max 15 characters)\nName: " ); get_input( players[0], 16 ); printf( "Thank you %s\n", players[0] ); printf( "Player 2 choose your name (max 15 characters)\nName: " ); get_input( players[1], 16 ); printf( "Thank you %s\n", players[1] ); } void reset_gamefield( char game_field[][7] ) { int i,j; for( i=0; i<7; i++ ) { for( j=0; j<7; j++ ) { game_field[i][j] = 2; } } } int try_move( int pid, int row, char game_field[][7] ) { if( row >= 7 or row < 0 ) { printf( "Wrong input. Please try again.\n" ); return 0; } int i; for( i=0; i<7; i++ ) { if( game_field[i][row] == 2 ) { game_field[i][row] = pid; return 1; } } printf( "Invalid move. Try again.\n" ); return 0; } int check_for_win( char game_field[][7], int row ) { int i=6; while( game_field[i][row] == 2 ) { i--; } int lrun = ((row-3)<0) ? (((row-2)<0) ? (((row-1)<0) ? 0 : 1) : 2) : 3; int rrun = ((row+3)>6) ? (((row+2)>6) ? (((row+1)>6) ? 0 : 1) : 2) : 3; int trun = ((i+3)>6) ? (((i+2)>6) ? (((i+1)>6) ? 0 : 1) : 2) : 3; int brun = ((i-3)<0) ? (((i-2)<0) ? (((i-1)<0) ? 0 : 1) : 2) : 3; int r; int count_m = 0; int count_dt = 0; int count_db = 0; for( r=0; r<(lrun+rrun+1); r++ ) { if( game_field[i][row] == game_field[i][(row-lrun)+r] ) { count_m++; if( count_m == 4 ) { return 1; } } else if( count_m != 0 ) { count_m--; } if( game_field[i][row] == game_field[(i-brun)+r][(row-lrun)+r] ) { count_db++; if( count_db == 4 ) { return 1; } } else if( count_db != 0 ) { count_db--; } if( game_field[i][row] == game_field[(i+trun)-r][(row-lrun)+r] ) { count_dt++; if( count_dt == 4 ) { return 1; } } else if( count_dt != 0 ) { count_dt--; } } count_m = 0; for( r=0; r<(trun+brun); r++ ) { if( game_field[i][row] == game_field[(i-brun)+r][row] ) { count_m++; if( count_m == 4 ) { return 1; } } else if( count_m != 0 ) { count_m--; } } return 0; } int play_game( char players[][16] ) { char game_field[7][7]; reset_gamefield( game_field ); // Choose who should start "random" static int rand_initialized = 0; if( !rand_initialized ) { /* According to http://stackoverflow.com/a/822368 srand should only be called once */ srand( time(NULL) ); rand_initialized = 1; } int current_id = rand() % 2; int game_over = 0; char input[5]; do { print_field( game_field ); do { printf( "%s's turn\n", players[current_id] ); printf( "Your choice [1-7]: " ); get_input( input, 5 ); if( !strncmp( input, "exit", 4 ) ) { return 0; } else if( !strncmp( input, "rest", 4 ) ) { return 1; } }while( !try_move( current_id, atoi( input ) - 1, game_field ) ); current_id = (current_id==1) ? 0 : 1; game_over = check_for_win( game_field, atoi( input ) - 1 ); }while( !game_over ); print_field( game_field ); printf( "%s Wins!\n", players[current_id] ); char again[2]; printf( "Want to play again?[Y/N]: " ); get_input( again, 2 ); if( strncmp( again, "Y", 1 ) == 0 or strncmp( again, "y", 1 ) == 0 ) { return 1; } return 0; } int main( ) { char players[2][16]; ask_players( players ); int running = 1; while( running ) { running = play_game( players ); } return 0; }Bin für jede Form von Kritik dankbar. Zumal ein Verweis auf mögliches Lehrmaterial zur Verbesserung von Laufzeitverhalten oder Memory Footprint auch nett wäre, falls da jemand was in petto hat.
Danke im voraus.
Gruß
-
fairiestoy schrieb:
C von A bis Z
Wirf es weg!
Siehe auch: https://www.c-plusplus.net/forum/272350Was stört dich an scanf? Immer die Standardfunktionen verwenden.
Ich habs mal getestet, deine check_for_win Funktion funktioniert nicht richtig, es wird immer der falsche Spieler als Gewinner ausgegeben.
-
- C von A-Z ist Schrott weil der Autor ein Pfuscher ist
- iso646 + and,or ist Nonsens, benutzt niemand
- srand gehört in main()
- strncmp ist Unsinn, entweder du vertraust auf dein get_input dass es immer gültige C-Strings liefert oder nicht
- get_input ist implementierungsmäßig Schrott u.a. wegen UB
- in Rechen-Funktionen (try_move) gehören keine stdout-Ausgaben
- der Gebrauch von input[5] sowohl für Strings wie für int ist designmäßig suboptimal
- die Spiele-Funktionen habe ich nicht geprüft, sie scheinen aber auch deutlich zuviel Code zu enthalten- prinzipiell ist dein Design OK
-> keine global. Variablen
-> Aufteilung in Funktionen mit geeigneten Parametern
-> die Hauptschleife in main()
-> Instanziierung eines Objektes in main() und Weitergabe an die Funktionen
-> Einhaltung des C-Standards, keine Verwendung von plattf.abh. Funktionen
-> wenn du schon C99 verwendest dann auch in for mit interner Variablendef.
-
Erstmal danke für die ersten Rückmeldungen.
Ich habs mal getestet, deine check_for_win Funktion funktioniert nicht richtig, es wird immer der falsche Spieler als Gewinner ausgegeben.
Hab's vorher immer mit das und asd als Namen getestet, da ist mir das nicht aufgefallen. War ein logisches Problem in der Umschaltung der User ID's. Ein einfaches '!' hat den Schaden behoben.
Was das Buch angeht, hätte ich wohl vorher besser recherchieren können. Jetzt ist es zu spät. Und unter den Bedingungen wohl auch schlecht mit gutem Gewissen wiederverkäuflich.
1 iso646 + and,or ist Nonsens, benutzt niemand
2 srand gehört in main()
3 strncmp ist Unsinn, entweder du vertraust auf dein get_input dass es immer gültige C-Strings liefert oder nicht
4 get_input ist implementierungsmäßig Schrott u.a. wegen UB
5 in Rechen-Funktionen (try_move) gehören keine stdout-Ausgaben
6 der Gebrauch von input[5] sowohl für Strings wie für int ist designmäßig suboptimal
7 wenn du schon C99 verwendest dann auch in for mit interner Variablendef.8 Was stört dich an scanf? Immer die Standardfunktionen verwenden.
- Entfernt, war mir nicht bewusst. Hab vorher eine Zeit lang mit Python herum gespielt und vermutlich daher diese Angewohnheit.
- Korrigiert. Erspart mir die static-Variable sowie den Prüfblock.
- Kannst näher erläutern, welcher Teil davon UB ist? Konnte mir jetzt so beim sinnieren über den Code höchsten die Anweisung betreffend des maximalen Buffer (255) als undefiniertes Verhalten erklären, weil nicht gewährleistet ist, dass mein Buffer groß genug für diese Anzahl an Bytes ist.
- Korrigiert mit einer separaten Variable für die Schleifenprüfung
- Sollte man hierfür denn zwei verschiedene Abfragen nehmen? Oder soll hier auch die Funktion scanf als Wrapper dienen?
- Korrigiert.
3+8) Mich würde nach ein wenig recherchieren (unter anderem https://en.wikipedia.org/wiki/Scanf_format_string#Security) interessieren, ob strings von scanf stets mit einem abschließenden \0 zurück gegeben werden? Hatte mich zuerst für getchar entschieden weil ich dieses Vorgehen (glaube ich) in den FAQ vom C-Board hier als ein Beispiel für die Leerung des Eingabepuffers gelesen hatte. Hielt dies für sinnig, vor allem weil ich nicht wusste, dass ich eine Längenangabe bei scanf verwenden kann.
Den Code würde ich gern erneut posten, wenn ich die ein oder andere Antwort auf meine Fragen gefunden habe. Finde es sonst unnütz jede Korrektur zu posten.
Danke nochmals für die Rückmeldungen.
Gruß
-
fairiestoy schrieb:
- Kannst näher erläutern, welcher Teil davon UB ist? Konnte mir jetzt so beim sinnieren über den Code höchsten die Anweisung betreffend des maximalen Buffer (255) als undefiniertes Verhalten erklären, weil nicht gewährleistet ist, dass mein Buffer groß genug für diese Anzahl an Bytes ist.
Du prüfst nirgendwo deine Länge! Wenn der Nutzer mehr als amount (oder gar BUFFER_MAX!) Zeichen eingibt, dann schreibst du fröhlich über das Ende hinaus. Das vorangehende memset ist hingegen reinstes Schlangenöl. Was bringt es vorher alles auf 0 zu setzen, wenn man dann als nächstes wieder etwas reinschreibt? Computerspeicher ist doch keine Tafel, die man erst abwischen muss, bevor man sie neu beschreiben kann.
- Sollte man hierfür denn zwei verschiedene Abfragen nehmen? Oder soll hier auch die Funktion scanf als Wrapper dienen?
scanf für so ziemlich alles hier. Dein Programm besteht zu großen Teilen daraus, scanf schlecht nachzumachen.
3+8) Mich würde nach ein wenig recherchieren (unter anderem https://en.wikipedia.org/wiki/Scanf_format_string#Security) interessieren, ob strings von scanf stets mit einem abschließenden \0 zurück gegeben werden? Hatte mich zuerst für getchar entschieden weil ich dieses Vorgehen (glaube ich) in den FAQ vom C-Board hier als ein Beispiel für die Leerung des Eingabepuffers gelesen hatte. Hielt dies für sinnig, vor allem weil ich nicht wusste, dass ich eine Längenangabe bei scanf verwenden kann.
Benutz eine vernünftige Referenz, zum Beispiel:
http://en.cppreference.com/w/c/io/fscanfs: matches a sequence of non-whitespace characters (a string)
If width specifier is used, matches up to width or until the first whitespace character, whichever appears first. Always stores a null character in addition to the characters matched (so the argument array must have room for at least width+1 characters)
-
void get_input( char buffer[], unsigned int amount ) { ... while( (tmp_c=getchar()) != EOF and tmp_c != '\n' ) { if( counter != amount ) { buffer[counter] = tmp_c; counter++; } } buffer[counter] = '\0'; }UB liegt vor, wenn du z.B. bei char input[5] eine Eingabe >4 Zeichen machst, dann ist counter == amount und mit
buffer[5] = '\0'greifst du auf undefinierten Speicher zu (hinter dem Array) == UB.
const char *getinput(char *s, int n) { /* liefert immer einen String und leert stdin; zusätzl. könnte man für zu lange Eingaben eine Fehlerbehandlung ergänzen */ char b[20]; assert(n > 1); sprintf(b, "%%%d[^\n]", n - 1); if (1 != scanf(b, s)) return ""; while (!ferror(stdin) && !feof(stdin) && getchar() != '\n'); return s; }- scanf liefert bei %s/%[] immer ein terminierendes '\0', wenn scanf das Feld ohne Fehler verarbeiten konnte
- die Eingabeauswertung ist deswegen suboptimal, weil auch hier wieder UB droht; bei Eingabe von "Exit" z.B. würde atoi() 0 liefern, 0-1 = -1 und du rechnest weiter mit -1 als Index, was sicher zu Unglück führt, hier fehlt also eine ausführliche Abfrage
-
Wutz schrieb:
void get_input( char buffer[], unsigned int amount ) { ... while( (tmp_c=getchar()) != EOF and tmp_c != '\n' ) { if( counter != amount ) { buffer[counter] = tmp_c; counter++; } } buffer[counter] = '\0'; }UB liegt vor, wenn du z.B. bei char input[5] eine Eingabe >4 Zeichen machst, dann ist counter == amount und mit
buffer[5] = '\0'greifst du auf undefinierten Speicher zu (hinter dem Array) == UB.
Eine Verständnisfrage. Habe ich hier nicht sogar zweimal UB provoziert? Einmal das Zugreifen das du bereits erwähnt hast, aber ist meine Überprüfung hier nicht per se auch schon falsch? Fiel mir beim drüber lesen auf.
while( (tmp_c=getchar()) != EOF and tmp_c != '\n' ) { if( counter != amount )Sobald ich amount erreicht habe, schreibt er dieses Byte zwar nicht mehr in den Buffer, aber danach allerdings schreibt er fröhlich weiter wenn die Eingabe > amount ist, oder vertue ich mich hier?
-
So ist es; habe ich doch auch schon gesagt. Ich hätte vielleicht nicht "nirgends" zu deiner Längenprüfung sagen sollen, sondern "falsch".
-
Moin,
wutz schrieb:
const char *getinput(char *s, int n) { /* liefert immer einen String und leert stdin; zusätzl. könnte man für zu lange Eingaben eine Fehlerbehandlung ergänzen */ char b[20]; assert(n > 1); sprintf(b, "%%%d[^\n]", n - 1); if (1 != scanf(b, s)) return ""; while (!ferror(stdin) && !feof(stdin) && getchar() != '\n'); return s; }- scanf liefert bei %s/%[] immer ein terminierendes '\0', wenn scanf das Feld ohne Fehler verarbeiten konnte
- die Eingabeauswertung ist deswegen suboptimal, weil auch hier wieder UB droht; bei Eingabe von "Exit" z.B. würde atoi() 0 liefern, 0-1 = -1 und du rechnest weiter mit -1 als Index, was sicher zu Unglück führt, hier fehlt also eine ausführliche AbfrageEs hat ein bisschen gedauert, bis ich die exakte Funktionsweise deines snippet erfasst habe. Wobei mich der Ausdruck
[^\n]noch immer irritiert, weil das nach einem RE aussieht. Konnte zu dieser Ausdrucksweise aber keine Referenzen finden wenn ich für diesen Ausdruck Google bemühe (außer Verweise auf Regex Libs). Hab auch http://en.cppreference.com/ die Output Funktionen durchforstet, allerdings finde ich dort auch nur die typischen Format specifier die immer genannt werden.
Kannst du mir bitte sagen, unter was für einem Stichwort das fällt damit ich danach suchen kann?
Zu deinem Einwand mit atoi gebe ich dir prinzipiell recht, dass ist designmäßig unschön. Allerdings habe ich zur Vorsicht (weil die Index -1 Rechnung schon vorgekommen ist) in Zeile 63 vom oben geposteten Code bewusst zumindest den Bereich der Eingabe überprüft, sodass (meiner Meinung nach) zumindest mit dem Index nicht mehr außerhalb des Speicherbereichs des Arrays zugegriffen werden kann. Oder habe ich hier was übersehen?
Geändert wird es trotzdem, allerdings geht es mir hauptsächlich ums Verstehen.
SeppJ schrieb:
So ist es; habe ich doch auch schon gesagt. Ich hätte vielleicht nicht "nirgends" zu deiner Längenprüfung sagen sollen, sondern "falsch".
Mein Verständnisfehler dann, Verzeihung. Hat einen Moment zum durchsickern gebraucht wie es scheint. ^^
Danke nochmal an euch für die Geduld.
-
Standard C hat keine RE.
Wenn du[^\n]nicht verstanden hast, hast du die Funktion auch nicht verstanden.
Suche mal unter
scanf scanset
-
Hallo fairiestoy,
es geht auch nicht um die Output-Funktionen, sondern um die Eingabefunktion scanf (s. unter Conversion specifier "[set]")
Die sprintf-Funktion baut nur den "format specifier" für scanf zusammen.
-
fairiestoy schrieb:
Allerdings habe ich zur Vorsicht (weil die Index -1 Rechnung schon vorgekommen ist) in Zeile 63 vom oben geposteten Code bewusst zumindest den Bereich der Eingabe überprüft, sodass (meiner Meinung nach) zumindest mit dem Index nicht mehr außerhalb des Speicherbereichs des Arrays zugegriffen werden kann. Oder habe ich hier was übersehen?
Nutzereingaben (und deren Eingabefehlerhandling) sind meist aufwändig, und insbesondere wenn sich die Kriterien mal ändern/erweitern nur aufwändig anzupassen.
Deshalb sieht man genau für solche Stellen dann Code vor, der lesbar/wartbar/erweiterbar ist und das nicht nur für den Original-Autor, z.B.:enum { RAUS, REST, INDEX, FEHLER }; int auswertung(const char*s) { if (!strcmp(s, "exit")) return RAUS; if (!strcmp(s, "rest")) return REST; if (strchr("1234567", *s) && !s[1]) return INDEX; return FEHLER; } ... int r; const char*a; do switch (r = auswertung(a = getinput((char[20]){ 0 }, 20))) { case RAUS: puts("exit"); break; case REST: puts("rest"); break; case INDEX: {int i = atoi(a); printf("%d", i); } break; case FEHLER: puts("fehler"); break; } while (r == FEHLER);
-
Wutz schrieb:
... int r; const char*a; do switch (r = auswertung(a = getinput((char[20]){ 0 }, 20))) { case RAUS: puts("exit"); break; case REST: puts("rest"); break; case INDEX: {int i = atoi(a); printf("%d", i); } break; case FEHLER: puts("fehler"); break; } while (r == FEHLER);Kleine Verständnisfrage zwischendurch:
Warum ist es hier erlaubt mit dem zurückgegebenen Zeiger (
a) vongetinput()weiter zu arbeiten? Erstreckt sich der Gültigkeitsbereich des temporären char[20] über das gesamte switch-statement?
-
Der Gültigkeitsbereich eines Objektes erstreckt sich (außer global) immer ab Definition bis zum Ende des Codeblocks/compound statement, hier also im Bereich von do-while und nicht switch.
-
Danke, ich hätte gedacht, dass das array nach dem Funktionsaufruf wieder zerstört wird, weil es nur ein temporary ist. Da habe ich mich dann wohl geirrt...
-
Wobei du jetzt im Gebrauch der Begriffe aufpassen musst: ein compound literal ist kein temporäres Objekt im Sinn des Standards, mit
temporary lifetimeist im Standard ganz was anderes gemeint.
-
Ups!
Ja, das hat mir gefehlt. Habe es recherchiert. Vielen Dank!
-
Ich danke auch.
Ja ja, kommt schon mal vor.
Ich bedanke mich für die qualifizierten Fragen, d.h. der Frager hat den Code mal wirklich durchdacht und nicht nur irgendwas aus Google oder sonstwo nachgeplappert.
