Quoridor

pmav99 · 5 Απριλίου 2016

Για το code style μπορείτε να χρησιμοποιείστε κάτι σαν το indent.

http://linux.die.net/man/1/indent

https://trac.osgeo.org/grass/wiki/Submitting/C#Indentation

imitheos · 5 Απριλίου 2016

* Καταρχάς παρατήστε την C. Όλα αυτά τα κατεβατά που γράψατε θα μπορούσαν να γραφτούν σε 3 γραμμές σε μια σοβαρή γλώσσα όπως η M2000.

* Αν πειστήκατε ότι η C δεν κάνει και αντί για την C σκέφτεστε να δουλέψετε C++, Java, C#, Python, κτλ, ΜΗΝ παρασύρεστε από παρωχημένες γλώσσες που έχουν ένα κάρο προβλήματα και δεν προσφέρουν τίποτα. Η καλύτερη γλώσσα που υπάρχει και η μόνη που αξίζει να ασχοληθείτε μαζί της είναι η M2000.

Αφού όμως πρόκειται για εργασία μαθήματος και είναι απαραίτητο να γίνει σε C, ας κάνουμε την καρδιά μας πέτρα και ας δούμε τι αλλαγές μπορούμε να προτείνουμε σε C

Το πρώτο πράγμα που βλέπει ο αναγνώστης τρέχοντας make είναι τα μηνύματα του compiler οπότε ας ξεκινήσουμε από αυτά. Στα αρχεία Utilities.c, main.c, playingCmds.c παίρνουμε προειδοποιητικό μήνυμα "implicit declaration of τάδε" δηλαδή δεν μπορεί να βρει την δήλωση κάποιων συναρτήσεων. Αυτό γίνεται γιατί δεν έχετε κάνει include κάποια από τα τοπικά αρχεία κεφαλίδων και το stdlib.h για την atoi.

Αυτό μπορεί να πάει ένα βήμα παραπέρα ενεργοποιώντας περαιτέρω προειδοποιήσεις. Πάντα να χρησιμοποιείτε την επιλογή -Wall ώστε να επιτρέψετε τον compiler να σας βοηθήσει όσο περισσότερο μπορεί, εντοπίζοντας περισσότερα πιθανά προβλήματα. Μία από τις προειδοποιήσεις που παίρνουμε η παρακάτω:

playingCmds.c: In function ‘playwall’:
playingCmds.c:91:29: προειδοποίηση: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
   else if (cmd[3] != NULL && strcmp(cmd[3],"vertical") == 0 || strcmp(cmd[3],"v") == 0)

Σου λέει ότι λόγω των κανόνων προτεραιοτήτων, μπορεί η παραπάνω έκφραση να μην κάνει αυτό που θέλετε. Για αυτό, καλό είναι να βάλεις παρενθέσεις ώστε να είσαι σίγουρος. Κάτι ακόμη καλύτερο είναι όταν έχουμε μια πολύπλοκη έκφραση, να την σπάμε σε μικρότερες ώστε να είναι πιο ευκολονόητη.

Άλλη βελτίωση που μπορεί να γίνει είναι στο συντακτικό κομμάτι. Να μπουν σχόλια στις δηλώσεις των συναρτήσεων ώστε να φαίνεται τι κάνουν και γιατί το κάνουν με αυτό τον τρόπο, να υπάρχει συνέπεια στις δηλώσεις (σε άλλες περιπτώσεις αφήνετε κενό μετά από τα κόμματα ενώ σε άλλες όχι), να μην υπάρχουν άχρηστες κενές γραμμές που δεν προσφέρουν τίποτα (συνήθως σε διπλωματικές το βλέπουμε αυτό για να μεγαλώσει ο αριθμός σελίδων ) και γενικά όσα σωστά σου πρότεινε ο Dr. Fuzzy.

Από άποψη υλοποίησης, ορίζονται κάποιες μεταβλητές και παίρνουν τιμή χωρίς να εξηγείται σε τι χρησιμεύει αυτό και επίσης λίγο παρακάτω χρησιμοποιείτε χύμα την τιμή αντί για την μεταβλητή που δηλώθηκε πριν ώστε να είναι δύσκολη η συντήρηση του κώδικα στο μέλλον.

    cmd = malloc(4*sizeof(char*));                                              
    for(i=0;i<4;i++){                                                           
        cmd[i] = malloc(50 * sizeof(char));

Για παράδειγμα, δεν θα ήταν πιο βολικό να μην είχαμε χύμα τα 4, 50 αλλά κάποιο #define ή κάτι παρόμοιο ? Επίσης, το sizeof(char) έχει πάντα τιμή 1 και μπορεί να παραλειφθεί.

Ορίζονται πολλές μεταβλητές μαζί στο ίδιο σημείο, χωρίς εξήγηση για την λειτουργία τους και με παρεμφερή ονόματα, πχ

char **board, command[50], **cmd;

Ο αναγνώστης θα δυσκολευτεί να καταλάβει την χρησιμότητα τους και τι διαφορά έχει η command με την cmd και έχει επιλεγεί να έχει διαφορετικό τύπο.

Μέσα στο σώμα του while(1) τι κάνει το printf("") ? Οι συναρτήσεις χρειάζεται να έχουν τόσα πολλά ορίσματα ή μπορούν να γραφούν διαφορετικά ? Όλα αυτά τα if-else με τα strcmp δεν μπορούν να γραφούν με πιο βέλτιστο τρόπο αντί για ένα μπερδευτικό κατεβατό ? Ενώ δηλαδή ο κώδικας κάνει κάτι πολύ απλό, μπερδεύεται ο αναγνώστης από τα τόσα πολλά if που μάλιστα έχουν μέσα δηλώσεις με χύμα νούμερα 4, 2, 50, κτλ. Σε ένα σημείο να σου ξεφύγει ένα 5 αντί για 4, θα ψάχνεις γιατί δεν παίζει σωστά.

Στο συγκεκριμένο πρόγραμμα δεν παίζει και τόσο μεγάλο ρόλο, αλλά γενικά να αποφεύγεις την atoi και λοιπές συναρτήσεις επειδή δεν σου παρέχουν έλεγχο και δεν έχεις εποπτία αν το αποτέλεσμα είναι σωστό.

M2000 · 5 Απριλίου 2016

Τα κατεβατά δεν τα γλιτώνει κανείς.Εφόσον είναι άσκηση οι επιλογές γλώσσας και τρόπος σύνταξης είναι του καθηγητή. Οπότε το ενδιαφέρον εδώ είναι η βελτιστοποίηση-συμμάζεμα.

π.χ. ο πίνακας position τη δουλεία έχει εδώ; Δηλαδή αυτή η συνάρτηση κάνει δυο ανεξάρτητα πράγματα!

void makeboard(char **board,int position[2][2],int sizeBoard) { 
77     int i,j; 
78 
 
79     position[0][0] = 1;                                     /*Position initialization.*/ 
80     position[0][1] = (4*sizeBoard) / 2; 
81     position[1][0] = (2*sizeBoard) - 1; 
82     position[1][1] = (4*sizeBoard) / 2; 
83 
 
84     for (i=0;i<2*sizeBoard + 1;i++){                        /*Filling the game board.*/ 
85         for (j=0;j<4*sizeBoard + 1;j++){ 
86             if (i % 2 == 0){ 
87                 if (j % 4 == 0){ 
88                     board[i][j] = '+'; 
89                 } 
90                 else{ 
91                     board[i][j] = '-'; 
92                     } 
93             } 
94             else{ 
95                 if (j % 4 == 0){ 
96                     board[i][j] = '|'; 
97                 } 
98                 else{ 
99                     board[i][j] = ' '; 
100                 } 
101             } 
102         } 
103     } 
104 board[1][((4*sizeBoard) / 2)] = 'B'; 
105 board[(2*sizeBoard) - 1][(4*sizeBoard) / 2] = 'W';          /*Ends here.*/ 
106 
 
107 }

kaliakman · 5 Απριλίου 2016

Αρχικά σας ευχαριστώ όλους πολύ που ασχοληθήκατε

Υλικό για διάβασμα:
- gitignore
- binary files in git

Will do!!

Δύο-τρια πραγματάκια που δεν έχουν να κάνουν με την υλοποίηση, αλλά για καλύτερο readability:

Βάλτε comment headers και γενικά comments στα functions/methods/classes κλπ στα αρχεία του project σας (κατά προτίμηση βάση του Doxygen https://www.stack.nl/~dimitri/doxygen/manual/docblocks.html#cppblock προκειμένου να μπορεί εύκολα να γίνει automatically documented το project σας).

Περάστε από έναν code beautifier τον κώδικα σας.

Επιλέξτε ένα naming convention (GNU, K&R, linux, κλπ) και ακολουθήστε το παντού (ονόματα αρχείων, μεταβλητές, ονόματα functions, κλπ), πχ, αποφασίστε ή capital ή mixedCase ή lowercase ή underscore filenames (Utilities.c, mainUtilities.h, minimax.c, κλπ) , αποφασίστε ή underscore ή mixedCase variables (list_head, currRow, κλπ);

Αυτό με τα headers θα το ψάξω γιατί όντως φαίνεται χρήσιμο εργαλείο. Όσο για το naming ο σκόπος ήταν mixedCase με το πρώτο μικρό αλλά προφανώς κάποια ξέφυγαν και θα διορθωθούν πάραυτα!!

Για το code style μπορείτε να χρησιμοποιείστε κάτι σαν το indent.

http://linux.die.net/man/1/indent

https://trac.osgeo.org/grass/wiki/Submitting/C#Indentation

Θα τα δοκιμάσω γιατί όπως παρατήρησαν και άλλοι υπάρχει μερικό inconsistency στο code style.

* Καταρχάς παρατήστε την C. Όλα αυτά τα κατεβατά που γράψατε θα μπορούσαν να γραφτούν σε 3 γραμμές σε μια σοβαρή γλώσσα όπως η M2000.

* Αν πειστήκατε ότι η C δεν κάνει και αντί για την C σκέφτεστε να δουλέψετε C++, Java, C#, Python, κτλ, ΜΗΝ παρασύρεστε από παρωχημένες γλώσσες που έχουν ένα κάρο προβλήματα και δεν προσφέρουν τίποτα. Η καλύτερη γλώσσα που υπάρχει και η μόνη που αξίζει να ασχοληθείτε μαζί της είναι η M2000.

Αφού όμως πρόκειται για εργασία μαθήματος και είναι απαραίτητο να γίνει σε C, ας κάνουμε την καρδιά μας πέτρα και ας δούμε τι αλλαγές μπορούμε να προτείνουμε σε C

Το πρώτο πράγμα που βλέπει ο αναγνώστης τρέχοντας make είναι τα μηνύματα του compiler οπότε ας ξεκινήσουμε από αυτά. Στα αρχεία Utilities.c, main.c, playingCmds.c παίρνουμε προειδοποιητικό μήνυμα "implicit declaration of τάδε" δηλαδή δεν μπορεί να βρει την δήλωση κάποιων συναρτήσεων. Αυτό γίνεται γιατί δεν έχετε κάνει include κάποια από τα τοπικά αρχεία κεφαλίδων και το stdlib.h για την atoi.

Με την εντολή make σου έβγαλε αυτά τα error? Γιατί μόνο το printf μου βγάζει όταν το τρέχω.

Αυτό μπορεί να πάει ένα βήμα παραπέρα ενεργοποιώντας περαιτέρω προειδοποιήσεις. Πάντα να χρησιμοποιείτε την επιλογή -Wall ώστε να επιτρέψετε τον compiler να σας βοηθήσει όσο περισσότερο μπορεί, εντοπίζοντας περισσότερα πιθανά προβλήματα. Μία από τις προειδοποιήσεις που παίρνουμε η παρακάτω:
playingCmds.c: In function ‘playwall’:
playingCmds.c:91:29: προειδοποίηση: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
   else if (cmd[3] != NULL && strcmp(cmd[3],"vertical") == 0 || strcmp(cmd[3],"v") == 0)
Σου λέει ότι λόγω των κανόνων προτεραιοτήτων, μπορεί η παραπάνω έκφραση να μην κάνει αυτό που θέλετε. Για αυτό, καλό είναι να βάλεις παρενθέσεις ώστε να είσαι σίγουρος. Κάτι ακόμη καλύτερο είναι όταν έχουμε μια πολύπλοκη έκφραση, να την σπάμε σε μικρότερες ώστε να είναι πιο ευκολονόητη.

Δίκιο έχεις αλλά εδώ πχ θεώρησα να είναι στην ίδια γραμμή ο έλεγχος είτε εισαχθεί ολόκληρη η λέξη ή το γράμμα μιας και είναι το ίδιο πράγμα.

Άλλη βελτίωση που μπορεί να γίνει είναι στο συντακτικό κομμάτι. Να μπουν σχόλια στις δηλώσεις των συναρτήσεων ώστε να φαίνεται τι κάνουν και γιατί το κάνουν με αυτό τον τρόπο, να υπάρχει συνέπεια στις δηλώσεις (σε άλλες περιπτώσεις αφήνετε κενό μετά από τα κόμματα ενώ σε άλλες όχι), να μην υπάρχουν άχρηστες κενές γραμμές που δεν προσφέρουν τίποτα (συνήθως σε διπλωματικές το βλέπουμε αυτό για να μεγαλώσει ο αριθμός σελίδων ) και γενικά όσα σωστά σου πρότεινε ο Dr. Fuzzy.

Εδώ θα υπάρξει βελτιώσει γιατί σκοπευώ να το περάσω από beautifier όπως προτεινε ο dr fuzzy και ο pmav99 μόλις γίνουν οι καταλλήλες διορθώσεις που προτάθηκαν εδώ.

Από άποψη υλοποίησης, ορίζονται κάποιες μεταβλητές και παίρνουν τιμή χωρίς να εξηγείται σε τι χρησιμεύει αυτό και επίσης λίγο παρακάτω χρησιμοποιείτε χύμα την τιμή αντί για την μεταβλητή που δηλώθηκε πριν ώστε να είναι δύσκολη η συντήρηση του κώδικα στο μέλλον.
    cmd = malloc(4*sizeof(char*));                                              
    for(i=0;i<4;i++){                                                           
        cmd[i] = malloc(50 * sizeof(char));      
Για παράδειγμα, δεν θα ήταν πιο βολικό να μην είχαμε χύμα τα 4, 50 αλλά κάποιο #define ή κάτι παρόμοιο ? Επίσης, το sizeof(char) έχει πάντα τιμή 1 και μπορεί να παραλειφθεί.

Εδώ έχεις δίκιο και φταίμε γιατί είχαμε στο μυαλό μας οτί ενώ σε εμάς makes sense θα είναι το ίδιο σε όλο.Θα φροντίσω να υπάρξουν τα κατάλληλα define για να αυξηθεί το readability!

Ορίζονται πολλές μεταβλητές μαζί στο ίδιο σημείο, χωρίς εξήγηση για την λειτουργία τους και με παρεμφερή ονόματα, πχ
char **board, command[50], **cmd;
Ο αναγνώστης θα δυσκολευτεί να καταλάβει την χρησιμότητα τους και τι διαφορά έχει η command με την cmd και έχει επιλεγεί να έχει διαφορετικό τύπο.

Θα ήταν καλύτερο να ορίζεται μια μεταβλητή ανά γραμμή και δίπλα να υπάρχει επεξήγηση δηλαδή? Θα το προσπαθήσω όσο περισσότερο μπορώ.

Μέσα στο σώμα του while(1) τι κάνει το printf("") ?

Στην αρχή δεν υπήρχε αλλά κάτι δεν γινόταν σωστά με το processing της εντολής του χρήστη αλλά δεν μπορώ με τίποτα να το θυμηθώ ( θα το ελέγξω πάντως γιατί γνωρίζω οτί χτυπάει ο compiler)

Οι συναρτήσεις χρειάζεται να έχουν τόσα πολλά ορίσματα ή μπορούν να γραφούν διαφορετικά ? Όλα αυτά τα if-else με τα strcmp δεν μπορούν να γραφούν με πιο βέλτιστο τρόπο αντί για ένα μπερδευτικό κατεβατό ? Ενώ δηλαδή ο κώδικας κάνει κάτι πολύ απλό, μπερδεύεται ο αναγνώστης από τα τόσα πολλά if που μάλιστα έχουν μέσα δηλώσεις με χύμα νούμερα 4, 2, 50, κτλ. Σε ένα σημείο να σου ξεφύγει ένα 5 αντί για 4, θα ψάχνεις γιατί δεν παίζει σωστά.

Αυτό μετά if-else στην main αναγνωρίζω οτί είναι πάρα πολύ άσχημο αλλά δυστυχώς δεν μπορέσαμε να σκεφτούμε κάτι πιο όμορφο ή λειτουργικό.Το μόνο που "βελτιώσαμε" είναι να υπάρχουν με την σειρά και το αριθμό των χρήσεων που γίνονται συνήθως (δηλαδή πάνω πάνω showboard playmove κτλ).

Για τα ορίσματα το μόνο καλύτερο που σκεφτήκαμε για να μειωθούν είναι να υπάρχει struct που θα περνάει αλλά θεωρήσαμε πως με τις αναθέσεις πριν την κλήση κάποιας συναρτήσης αλλά και μόλις καλεστεί σε τοπικές(για να αποφευχθεί αυτό που είπα και στον Μ2000) θα μειωθεί η αναγνωσιμότητα.

Όσο για αυτό με τα σκόρπια νούμερα μπορείς να γίνεις λίγο πιο συγκεκριμένος ως προς την τοποθεσία που το παρατήρησες; Πάντως νομίζω και αυτό μπορεί να επιλυθεί με σχόλια δίπλα από τις αναθέσεις.

Στο συγκεκριμένο πρόγραμμα δεν παίζει και τόσο μεγάλο ρόλο, αλλά γενικά να αποφεύγεις την atoi και λοιπές συναρτήσεις επειδή δεν σου παρέχουν έλεγχο και δεν έχεις εποπτία αν το αποτέλεσμα είναι σωστό.

Στις atoi και γενικά στην είσοδο έχουμε προσπαθήσει να έχουμε ενα υποτυπώδες error handling αλλά μήπως θα μπορούσες να μου πεις κάποιον άλλο τρόπο να γίνει αυτή η επεξεργασία καλύτερα;

Και πάλι σας ευχαριστώ όλους και ειδικά τον imitheos για την βοήθεια σας!!

Τα κατεβατά δεν τα γλιτώνει κανείς.Εφόσον είναι άσκηση οι επιλογές γλώσσας και τρόπος σύνταξης είναι του καθηγητή. Οπότε το ενδιαφέρον εδώ είναι η βελτιστοποίηση-συμμάζεμα.

π.χ. ο πίνακας position τη δουλεία έχει εδώ; Δηλαδή αυτή η συνάρτηση κάνει δυο ανεξάρτητα πράγματα!

Η συνάρτηση έχει ως σκοπό να setάρει το ταμπλό του παιχνιδιού που πέρα από την δημιουργία του "γραφικού περιβάλλοντος"(Ο Θεός να το κάνει!) περιλαμβάνει και τους παίκτες όποτε θεωρήσαμε ως πρέπον να οριστούν εδώ και οι θέσεις τους αφου ουσιαστικά εδώ γίνεται η προετοιμασία για το παιχνίδι.

Kercyn · 6 Απριλίου 2016

Μπράβο και στους δύο για την εργασία σας.

Βλέπω ότι η κατηγοριοποίηση που έχεις κάνει στα αρχεία σου έχει να κάνει με τα source files σου (δηλαδή έχεις ένα Utilities σκέτο, ένα Utilities για τη main κι άλλο ένα για το BFS). Νομίζω μια πιο "κατανοητή" κατηγοριοποίηση θα ήταν να βάζεις πράγματα που ασχολούνται με μια κατηγορία στο ίδιο αρχείο και να του δώσεις ένα περιγραφικό όνομα, έτσι ώστε όποιος το ανοίγει να ξέρει τι θα βρει μέσα (και ανάποδα, κάποιος που ψάχνει να βρει μια συνάρτηση που κάνει κάτι να ξέρει πού να ψάξει).

Γενικά μου είναι λίγο δύσκολο να διαβάσω όλο τον κώδικα, κυρίως γιατί δεν υπάρχει κάτι που να εξηγεί πώς δουλεύει το όλο πρόγραμμα. Δε μιλάω για το τι κάνει κάθε συνάρτηση ξεχωριστά αλλά για το πώς είναι δομημένο όλο το project.

Και να κάνω μια ερώτηση κι εγώ προς τους υπόλοιπους.

Πόσο viable/καλό/readable θα ήταν να έχει ένα struct το οποίο θα έχει ένα μέσα ένα char* (το key του command) και ένα variadic function pointer που θα έδειχνε στο function του εκάστοτε command; Αν δε σας αρέσει το struct, έναν πίνακα από variadic function pointers και τα κατάλληλα defines για τα indices (define CMD_FUNC1 0, define CMD_FUNC2 1 κλπ).

kaliakman · 6 Απριλίου 2016

Μπράβο και στους δύο για την εργασία σας.

Βλέπω ότι η κατηγοριοποίηση που έχεις κάνει στα αρχεία σου έχει να κάνει με τα source files σου (δηλαδή έχεις ένα Utilities σκέτο, ένα Utilities για τη main κι άλλο ένα για το BFS). Νομίζω μια πιο "κατανοητή" κατηγοριοποίηση θα ήταν να βάζεις πράγματα που ασχολούνται με μια κατηγορία στο ίδιο αρχείο και να του δώσεις ένα περιγραφικό όνομα, έτσι ώστε όποιος το ανοίγει να ξέρει τι θα βρει μέσα (και ανάποδα, κάποιος που ψάχνει να βρει μια συνάρτηση που κάνει κάτι να ξέρει πού να ψάξει).

Γενικά μου είναι λίγο δύσκολο να διαβάσω όλο τον κώδικα, κυρίως γιατί δεν υπάρχει κάτι που να εξηγεί πώς δουλεύει το όλο πρόγραμμα. Δε μιλάω για το τι κάνει κάθε συνάρτηση ξεχωριστά αλλά για το πώς είναι δομημένο όλο το project.

Και να κάνω μια ερώτηση κι εγώ προς τους υπόλοιπους.

Πόσο viable/καλό/readable θα ήταν να έχει ένα struct το οποίο θα έχει ένα μέσα ένα char* (το key του command) και ένα variadic function pointer που θα έδειχνε στο function του εκάστοτε command; Αν δε σας αρέσει το struct, έναν πίνακα από variadic function pointers και τα κατάλληλα defines για τα indices (define CMD_FUNC1 0, define CMD_FUNC2 1 κλπ).

Σε ευχαριστώ πολύ! Θα κάτσω και ελπίζω μέχρι το σαββατοκύριακο ελπίζω να έχω κάνεις τις απαραίτητες αλλαγές!

Σύνδεση

Quoridor

Προτεινόμενες αναρτήσεις

pmav99

imitheos

M2000

kaliakman

Kercyn

kaliakman

Δημιουργήστε ένα λογαριασμό ή συνδεθείτε για να σχολιάσετε

Δημιουργία λογαριασμού

Σύνδεση

Σύνδεση