Conversation
daniefer
left a comment
daniefer
left a comment
There was a problem hiding this comment.
All this feedback is more of design feedback and less of functionality feedback. In programing there are 1000s of ways to solve the same problem. What separates good from great is making it easy to understand and change later. So keep up the good work!
| //Console.ReadLine(); | ||
| } | ||
|
|
||
| public static void GetInput() |
There was a problem hiding this comment.
This methods does a lot more than just "get input". Try to keep your method names in sync with what they actually do. In this case, I would split this into a GetInput, PlaceMark (which you already have), CheckForEndGame, and TogglePlayer methods. Then in your Program.Main method, call them in the right order.
| if (HorizontalWin() || VerticalWin() || DiagonalWin() == true) | ||
| { | ||
| Console.WriteLine("Player " + playerTurn + " Won!"); | ||
| Environment.Exit(0); |
There was a problem hiding this comment.
No need to call Environment.Exit here, just return true and your while loop should exit
|
|
||
| } | ||
|
|
||
| public static bool CheckForTie() |
There was a problem hiding this comment.
This method name says to me, "you can call me to determine if the game has ended in a tie". But what it is actually doing is checking if any spots remain unplayed. There is a distinctions there that could lead to trouble for someone else who does not know you wrote this method with the assumption it would always be called after checking for no wins.
|
|
||
| public static bool CheckForTie() | ||
| { | ||
| if (board[0][0] != " " && board[0][1] != " " && board[0][2] != " " && board[1][0] != " " && |
There was a problem hiding this comment.
Good job!
To put it another way though, you want to check if there are any remaining unplayed spots. There is a way to do this with loops that might lend itself better to understanding what you are wanting to confirm vs checking every spot explicitly (by setting the next of each array). Just think about it some and if you have time to go back to this later, see if you can rework it.
Sort/Program.cs
Outdated
| //Sorting numbers from smallest to largest// | ||
| int[] numArray = { 23, 55, 99, 38, 65 }; | ||
|
|
||
| for (int i = 0; i < numArray.Length - 1; i++) |
There was a problem hiding this comment.
This sort needs to work for any int[] I set this too. When you find yourself copying and pasting code to do something more than once that means you probably need a while loop. You need to be able to say, "sort this array of integers until all integers are sorted". Right now, if you attempt to sort the array [5, 4, 3, 2, 1] your output is not sorted.
Try to convert this into a loop that does just that, continue sorting until the array is sorted.
2 participants
No description provided.