חוות דעת על איקס עיגול ב-C#

[AvengersAssemble]

בניתי Console Application של איקס עיגול שבוע שעבר וחבר סיפר לי שיש הרבה אנשים מבינים באתר הזה, אז חשבתי לפרסם ולשמוע מה דעתכם. אני לומד C# רק שבועיים אז אל תצפו למשהו מדהים, עם זאת אשמח אם תפרטו לי כמה שיותר מה אפשר לשפר בשביל שהתכנית תהיה יותר יעילה ומובנת ולמה.

התכנית: http://pastebin.com/M67nXUam#

כל שחקן מכניס בתורו מספר שורה ועמודה, התכנית מכריזה אם יש מנצח או תיקו, בודקת שאין ניסיון לשכתוב "משבצת" שכבר שובץ בה X/O, מדפיסה בתחילת כל משחק את מספר המשחק והתוצאה (נקודות של X, נקודות של O, מספר משחקים שנגמרו בתיקו).

ועוד שאלה:

את הלולאה שאין ניסיון שכתוב (רמאות) כתבתי בתור לולאת while, אבל מכיוון שהיא בכל מקרה חייבת לרוץ פעם אחת ואז אם התנאי עדיין מתקיים לרוץ שוב ואם לא לצאת חשבתי לכתוב אותה בתור לולאת do. הנה קטע מהתכנית שבודק רמאות, פעם אחת בעזרת לולאת while ופעם אחת בעזרת לולאת do:

http://pastebin.com/hQLyAvqj

אשמח אם תעזרו לי לבחור בדרך היעילה יותר, ותסבירו למה.

תודה לעונים

[שניצל]

קודם כל הערה כללית:

כשיש לך if/for/while, גם אם הגוף הוא רק של שורה אחת, עדיין תעטוף אותו בסוגריים מסולסלים. כלומר, אל תעשה ככה:

if (blabla)
  blablabla;
else
  blablablabla;

אלא ככה:

if (blabla) {
  blablabla;
} else {
  blablablabla;
}

זה הופך את הקוד ליותר קריא ופחות מועד לטעויות.

ועכשיו לבעיות ספציפיות. קודם כל השורה הזו:

for (int k = 1; k > 0; k++)

מה המשמעות של התנאי הזה? אם k רק הולך וגדל הוא תמיד יהיה גדול מ-0. אתה יכול פשוט להשמיט את התנאי, או לחילופין להחליף לחלוטין את הלולאה בלולאת while. אתה יכול גם לתת תנאי עצירה אמיתי - נגיד, להחזיק משתנה שיאמר האם המשחק הנוכחי הסתיים או לא. בכלל, אתה לא משתמש ב-k מעבר לקביעת מי השחקן הנוכחי, אז יכולת פשוט להחליף אותו ב-bool שמתחלף בכל תור.

אין בעיה עם שתי הצורות של ה-while, בדרך כלל משתמשים ב-while ולא ב-do-while כי זה יותר קריא (ברור מההתחלה מה תנאי העצירה). אני הייתי קורא למשתנה validSelection ומאתחל ל-false ואז עושה (while(!validSelection - זה הרבה יותר קריא ("כל עוד לא התבצעה בחירה חוקית"). בכל מקרה, עדיף לאתחל את המשתנה מיד לפני הכניסה ללולאה, ולא מיד אחריה.

למה Winner מחזירה int? היא מחזירה רק האם היה ניצחון או לא. יותר הגיוני שתחזיר bool. גם השם שלה לא הכי אינפורמטיבי.

הפונקציה PrintBoard מאוד מסורבלת. למה היא לא בנויה כלולאה בתוך לולאה, כמו SetBoard? חוץ מזה, כדאי שתכיר את האופרטור % שמבצע חילוק ומחזיר את השארית.

הפונקציה IsTie קצת מוזרה. קודם כל, אתה מגדיר משתנה בשם notTie ואז מחזיר את ההיפך שלו? למה לא מלכתחילה לקרוא לו tie (ולשים לו ערכים בהתאם) ולהחזיר אותו כמו שהוא? חוץ מזה, הוא לא נחוץ בכלל. אפשר לצאת באמצע הפונקציה באמצעות return וזהו (ואז גם לא צריך break).

מה עושה הפונקציה Result? מעדכנת את התוצאה, או מדפיסה אותה? עדיף להפריד לשתי פונקציות יותר ברורות.

[AvengersAssemble]

קודם כל, תודה על התגובה המפורטת

רשמתי את הקוד מחדש, בהתאם להערות שלך: http://pastebin.com/xpT9VdLQ

במקור, הפונקציה Result הן מעדכנת והן מדפיסה את התוצאה, אבל בגרסא הנוכחית היא רק מעדכנת את התוצאה. פקודה אחרת שבאה מיד לאחר הקריאה לפונקציה בפונקציה Main היא זו שמדפיסה את התוצאה.

Winner לא החזירה int מסיבה מסוימת, פשוט בדיוק סיימתי את הנושא של פונקציות מהמדריך של Webmaster ורציתי לתרגל פונקציות שמחזירות ערכים מטיפוסים שונים. שיניתי ל-bool. השם שלה הוא isWinner, בהתאמה ל-isTie.

כשבניתי את PrintBoard, לא חשבתי (לא יודע איך זה קרה) על ליצור אותה כמו SetBoard. כמו שאר הדברים שהערת עליהם, תיקנתי את זה ובקוד החדש הפונקציה PrintBoard בנויה כלולאה כלואה בלולאה.

באשר לאופרטור % - אני לא בטוח באיזה הקשר רשמת את הערה עליו, אבל אני מכיר אותו ונעזרתי בו בקוד המקורי בכדי לקבוע מי השחקן, כמו שראית והערת.

השם שהצעת למשתנה באמת ברור יותר, אז שיניתי (גם ההערות בלולאת ה-while הזו ברורות יותר עכשיו). את לולאת ה-for החלפתי בלולאת while, כמו שהצעת. בהחלט הופך את הקוד ליותר מובן וקריא.

בנוגע לסוגריים המסולסלים - שיניתי את זה בגרסא הנוכחית, אבל אני לא מבין למה זה כל כך חשוב כל עוד אני מקפיד על כתיבה הדרגתית.

אשמח אם תעבור על הקוד שוב ותרשום אם יש לך הערות נוספות .

[שניצל]

במקור, הפונקציה Result הן מעדכנת והן מדפיסה את התוצאה, אבל בגרסא הנוכחית היא רק מעדכנת את התוצאה. פקודה אחרת שבאה מיד לאחר הקריאה לפונקציה בפונקציה Main היא זו שמדפיסה את התוצאה.

יותר טוב, אבל השם של הפונקציה Result לא אינפורמטיבי (לא ברור מהשם מה היא עושה).

באשר לאופרטור % - אני לא בטוח באיזה הקשר רשמת את הערה עליו, אבל אני מכיר אותו ונעזרתי בו בקוד המקורי בכדי לקבוע מי השחקן, כמו שראית והערת.

התכוונתי לזה בהקשר של PrintBoard. בדקת אם i==0 או i==3 או i==6, כשיכולת פשוט לבדוק אם i%3 == 0.

בנוגע לסוגריים המסולסלים - שיניתי את זה בגרסא הנוכחית, אבל אני לא מבין למה זה כל כך חשוב כל עוד אני מקפיד על כתיבה הדרגתית.

זה לא חובה, זה עניין של קונבנציה. לדעתי זה קריא יותר ככה. חוץ מזה, זה פחות מועד לטעויות - אם אתה כותב if בלי סוגריים מסולסלים, ואחר כך אתה רוצה להוסיף עוד שורה לגוף של התנאי, אתה יכול בטעות להוסיף את השורה בלי לשים לב שאין שם סוגריים מסולסלים ואז השורה שהוספת תהיה מחוץ ל-if.

הקוד נראה יותר טוב עכשיו. בפונקציה isTie, המשתנה tie לא נחוץ כלל, ואתה יכול להשמיט אותו. חוץ מזה, אפשר לבדוק שהגיע תיקו בדרך הרבה יותר פשוטה מאשר לעבור על הלוח (מה שדי מייתר את קיומה של הפונקציה isTie).

חוץ מזה, כמו שאמרתי בהודעה הקודמת, יותר יפה שתעשה validSelection = false מיד לפני השימוש בו, ולא מיד אחרי.

[AvengersAssemble]

יותר טוב, אבל השם של הפונקציה Result לא אינפורמטיבי (לא ברור מהשם מה היא עושה).

התכוונתי לזה בהקשר של PrintBoard. בדקת אם i==0 או i==3 או i==6, כשיכולת פשוט לבדוק אם i%3 == 0.

זה לא חובה, זה עניין של קונבנציה. לדעתי זה קריא יותר ככה. חוץ מזה, זה פחות מועד לטעויות - אם אתה כותב if בלי סוגריים מסולסלים, ואחר כך אתה רוצה להוסיף עוד שורה לגוף של התנאי, אתה יכול בטעות להוסיף את השורה בלי לשים לב שאין שם סוגריים מסולסלים ואז השורה שהוספת תהיה מחוץ ל-if.

הקוד נראה יותר טוב עכשיו. בפונקציה isTie, המשתנה tie לא נחוץ כלל, ואתה יכול להשמיט אותו. חוץ מזה, אפשר לבדוק שהגיע תיקו בדרך הרבה יותר פשוטה מאשר לעבור על הלוח (מה שדי מייתר את קיומה של הפונקציה isTie).

חוץ מזה, כמו שאמרתי בהודעה הקודמת, יותר יפה שתעשה validSelection = false מיד לפני השימוש בו, ולא מיד אחרי.

שוב תודה

הקוד המעודכן: http://pastebin.com/mqcuEYAb

שינויים: הורדת המשתנה tie, validSelection = false לפני השימוש, שינוי שם הפונקציה Result ל-UpdateResult.

אני אחשוב איך לבדוק אם הגיע תיקו בלי לעבור על הלוח כבר מחר, אז אל תגלה לי

מה שבאמת מציק לי זה הפונקציה isWinner, לעבור על כל קומבינציה אפשרית לניצחון נראה כמו דרך לא אלגנטית להחריד לקביעת המנצח ויהיה סיוט לשנות את זה ללוח של 4X4 לדוגמא. אני אעבור על זה עכשיו ואנסה להכנס את זה ללולאה.

[שניצל]

אתה יכול להחליף את המימוש של isWinner בכמה לולאות.

[Gil28]

מה שבאמת מציק לי זה הפונקציה isWinner, לעבור על כל קומבינציה אפשרית לניצחון נראה כמו דרך לא אלגנטית להחריד לקביעת המנצח ויהיה סיוט לשנות את זה ללוח של 4X4 לדוגמא. אני אעבור על זה עכשיו ואנסה להכנס את זה ללולאה.

שים לב ששחקן יכול לנצח רק מיד אחרי התור שלו. כלומר, אחרי שהוא סימן X (או O) במשבצת מסויימת, אם הוא ניצח המשבצת הזאת היא חלק מהניצחון.