31

This is not bad for a programmer as new to C++ as you have said you are. Keep up the good work! With that said, here are some ideas on how you might be able to improve your program. Don't abuse using namespace std Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It's not necessarily wrong to use it, but ...


25

Here are some things that may help you improve your code. Use the required #includes The code uses std::string which means that it should #include <string>. It was not difficult to infer, but it helps reviewers if the code is complete. Have you run a spell check on comments? If you run a spell check on your comments, you'll find a number of things ...


18

First of all: nice work! It's easy to read and understand. Program organization It's very good that you split the task to small functions. Reading the body of main reveals nicely the overall flow. Ideas for further improvement: place does two things: it reads input from user and updates the state of the board. It would be good to separate these logically ...


10

There's a typo in this line: }else if(arr[1]+arr[4]+arr[7]==3*10||arr[1]+arr[7]+arr[7]==3*11){ //win condition outputs the winner player's name for all I think that the second 7 was intended to be 4. That's the kind of thing that tends to happen when you repeat large blocks of similar code. And most readers won't spot that. What you should do is ...


9

There's two things I'd change to simplify your code: Use a two dimensional array. Sure you can do the conversion from a 2d position to 1d easily, but the compiler can do and board[1][1] is rather more obviously the middle of the board than board[4]. Instead of hardcoding your logic for what positions you have to check, think about a programmatic approach ...


8

In addition to Edwards answer, there is a bug in check_diagonals. The first check for the '\' diagonal should check for positions 0 and 8. I think you also switched up the names for check_horizontal and check_vertical, since check_vertical effectively checks the rows and check_horziontal checks columns.


8

I think youre code is quite nice. Easy to read and the tasks are nicely splitted into functions. So theres not much to add. Theres just one thing bothering me. Don't omit curly braces. This: // Check rows if (b[0] == p && b[1] == p && b[2] == p) return true; if (b[3] == p && b[4] == p && b[5] == p) return true; if (...


8

Sorry, but that someone is probably right. At the same time, thanks for being thus brave and asking on how to improve here. It's not so simple to give feedback in such a scenario, since there are so many details that go "wrong". Since you tagged this for beginner, I'll focus on things that will benefit you most as well as things you can easily adapt. You ...


7

The following remarks may be nitpicks. Since your code is already quite good, it's all I have to say. :) #include <stdio.h> #include <stdlib.h> #include <stdbool.h> Since all the above headers are from the standard C library, they should be in alphabetical order. When you include other headers like <sys/type.h>, the order is ...


7

Nice job using ENUMS 👍 I'm going to try to do the Code Review top-down. Don't shorten variable names The compiler doesn't care how long names are. I'm guessing the 'hm' in hmInARowToWin stands for 'howMany'. So it should be changed to howManyInARowToIn or amountInARowToWin. Use 'final' keyword when possible Your scanner object is never going to change....


7

void main() is not portable Although it "works" in a lot of cases, the only two portable definitions of main are: int main(void) and int main(int argc, char **argv) Error checking You don't check the return value of scanf. This would be better: if(scanf("%d", &n) != 1) { fputs("Error; enter a number.", stderr); return EXIT_FAILURE; /* ...


6

Please add some space to your code - it can be hard to read when it's all crammed together. For example, compare these two versions of the same function; first, as written: //function to find the number of available available_spots int available_spots(char (*GRID)[7]) { int spots=0; for(int i=1;i<6;i=i+2) for(int j=1;j<6;j=j+2) { if (GRID[...


6

First, congratulations for writing a complete program. It's certainly the best way to learn, and the most gratifying. But I would also say that it's only the beginning: it's time to look back at your code, at the creative process, and think about what you could do to make your code more maintainable, and easier to improve. I believe that your code should be ...


6

The code looks clean and readable. You could rename check to checkForWin to tell what it checks. I would also suggest to improve that procedure by declaring the lines as a constant array instad of writing a similar test 8 times: const int lines[8][3] = { { 0, 1, 2 }, // rows { 3, 4, 5 }, { 6, 7, 8 }, { 0, 3, 6 }, // columns { 1, 4, 7 },...


6

Event Dispatching Thread Since Swing is not thread safe, all Swing UI component creation and modification should be done on the Event Dispatching Thread (EDT). It isn’t a problem with this program, but you could run into it if you also created (say) a javax.swing.Timer in your initiation. It is an easy change to create the GUI on the EDT. Instead of Gui ...


5

While your goal may be to "make it so the reader can understand the current method they are reading", I actually found it very confusing to read the code. Consider: from games_engine import GamesEngine from report_on_game import ReportOnGame from report_on_many_games import ReportOnManyGames Ok, so games_engine, report_on_game, and report_on_many_games ...


5

First, congratulations on getting it to work. Most falter earlier. Now, let's see what you can improve: Generally, you use multiple files to enable separate compilation, and in line with that you don't #include source-files, only header-files. Please, please ensure your files are consistently lower-case, even if Windows doesn't care. It's not quite as bad ...


5

Computing a square-root is unnecessary; further, it only works when kBoardSize is a perfect square. Prefer const int kBoardLength = 3; const int kBoardSize = kBoardLength * kBoardLength; Your test for unreachability is not complete. You verify that O has made no more moves than X, but you really need to check that the winner made the last move. If X won, ...


5

Style and code Numeric styles can be set using Number. eg modalParent.style.opacity = '1'; can be modalParent.style.opacity = 1; To convert a string to a number use Number rather than parseInt, or coerce the value eg const foo = "1" * 1 Good code style's most important attribute is consistency. If you use a particular style use the same style throughout the ...


5

and welcome, and thanks for providing an interesting subject. I like the idea of difficulty levels for the game! That said, your code is too long. Not "too long to review," just "too long." Moar functions! (& Generators!) Consider this: for i in range(size): for j in range(size): if board[i][j] != ai and board[i][j] != pl: ...


5

Note, some of these require a build process the iife should be added in the build process, it's distracting The code doesn't expose anything. It's untestable. var should be let or const You can use arrow functions to turn some of the functions into one liners Inconsistent formatting - I recommend getting a linter setApplicationView - Should probably be ...


5

This is a code inspection so it's my role to raise questions, not to answer them. I haven't run your code. Did you check your end condition? It appears to me that the first player, let's say it's x, goes on turn_count 0, 2, 4, 6, 8. In the loop when turn_count is 8 you will accept input from x, place it on the board, then increment turn_count to 9 and break ...


5

malloc I start with one of the most common ones. Instead of char *board = malloc(sizeof(char) * BOARD_SIZE) write char *board = malloc(sizeof(*board) * BOARD_SIZE). If you decide to change the type in the future, you don't have to change at more than one place. And besides, sizeof(char) is ALWAYS 1. But the biggest problem is that you're not checking the ...


5

Ways to improve: simplifying list construction: *list(x if x != '0' else ' ' for x in board)) --> *[x if x != '0' else ' ' for x in board] check_diagonal function. The function performs a logical check if the passed row is fully selected/marked with the same mark/code and returns boolean result respectively.Therefore, it's better to give it a more ...


5

First of all, I suggest you read pep8 (https://pep8.org/). I see you have violations. Understanding pep8 is a must if you want to be a pro Python developer. Better yet, use flake8 to automatically find pep8 violations in your code. Second, using that many elif statements is always a red flag, especially when they seem to be doing the same thing. Can you ...


4

Here are a number of things that may help you improve your code. Don't abuse using namespace std Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It's especially bad to use it when writing headers. Separate interface from implementation The interface goes into a header file and the implementation (that ...


4

(Note: no code in this post has been run- beware errors.) I have a couple of issues with your design, and with your review request. First, you say that you are committed to using five different functions for the checking. That's unfortunate, since this problem is very amenable to being solved with iteration. Next, you have a class GameOutcome that is ...


4

Naming The enum Cell defines Empty, Circle and Cross. This values aren't cells in terms of an object, they are more like types. For example: Empty is not a cell, but a cell can be empty You can rename Cell to CellType to make it more clear. Comments Comments are a good documentation. You did a great job with the <summary>-tag. But the comments ...


Only top voted, non community-wiki answers of a minimum length are eligible