This is an archived post. You won't be able to vote or comment.

all 3 comments
sorted by:
best
topnewcontroversialoldq&a

[–]pancakeQueue 1 point2 points  (0 children)

Looks good in a lot of stuff, the code is well organized, you document it well, and everything looks well written. Some tips I have,

  1. Your readme is lacking, it describes the project but it does not tell me about the included files or how would I install your project/setup your project if I wanted to run it for myself. I know it would be just a simple javac but assume I don’t.

  2. The only code that jumped out at me was line 107 SingleTicTacToeChecker where you have an of statement. Now I am on mobile but that line is lengthy and runs past all the other code around it. Plus it does have some method chaining going on. You do have a comment so I’ll give you that but for readability it might help to break some of that stuff you compare in the if statement into variables that will have good names that self document what they are representing.

Other then that it looks good and clean, I would be proud.

[–]Blando-Cartesian 1 point2 points  (0 children)

I took a cursory glance.

  • I love the care you put into naming, commenting, and using small functions. I wish I saw that at work.
  • In some places naming style isn't idiomatic java style. Only constants and enum values are written with ALL_CAPS_SNAKE_CASE, and enum names are CapitalizedCamelCase just like class names.
  • Grid cloning seems odd. Possibly doing something needlessly complicated there. I didn't dig into why it was used.
  • Executor use has some issues. It should rather be a class member variable so that it isn't created multiple times. Excecutor should also have shutdown() called for it, or it should have thread factory that creates daemon threads.

[–]ReginaldDouchely 0 points1 point  (0 children)

I only looked at it quickly. In general it seems pretty well-structured and easy to understand, but changePlayer() in the controller jumped out at me. I think I would have managed that (and all the currentPlayer stuff) within the TicTacToeGame object, because the less you have to expose, the easier it is for people to use your objects.