Please critique Brad's tic-tac-toe game!

Project Feedback
#1

Hi coders!

This is my tic-tac-toe game here : https://blbaylis.github.io/tic-tac-toe. I want to use this in my portfolio, so I tried to go the extra mile.

What do you like about it? What do you dislike about it? How could it be improved?

Any feedback is very welcome!

1 Like
#2

First of all, it’s awesome that you can choose the colour and style of the player avatars. The game plays as it should; there doesn’t seem to be a problem with the game itself. So fantastic work on both those parts!

There appears to be a case where the avatars don’t work as they should. I did these steps from the start:

  • Select any of the crosses, any colour. Confirm selection. The red plain circle should be selected for the second player.
  • Hit back, then select any circle, any colour. Confirm selection. The red cross should now be selected for the second player.
  • However, it does not let me continue at this point, even though it looks like both players have a viable avatar.
  • It fixes if I actually select an avatar for the second player. I think that although it shows a selected avatar in the box, the game thinks no avatar has been selected which is why it doesn’t continue.

*Edit: Just as a feature request, maybe after restarting a game it could ask you if you’d like to go first or second?

1 Like
#3

Let me just first say I believe the overall visual design is fantastic! Now the bad news, the JavaScript code is loaded with code sections which are not DRY (Do Not Repeat Yourself).

For example, the following:

document.getElementsByClassName("confirm-btn")[0].addEventListener("click", function(event){
  document.getElementsByClassName("flipper")[0].style.transition = "0.6s";
  flip();
  userIcon.updateIcon("user-square");
  if (gameObj.firstMove === "user"){
    document.getElementsByClassName("open-icon-select-btn")[0].style.backgroundImage = 
	  "url('icons/" + userIcon.icon + "/" + userIcon.icon + " " + userIcon.colour + ".png')";
  } else {
    document.getElementsByClassName("open-icon-select-btn")[1].style.backgroundImage = 
	  "url('icons/" + userIcon.icon + "/" + userIcon.icon + " " + userIcon.colour + ".png')";
  }
});

could be replaced with:

document.getElementsByClassName("confirm-btn")[0].addEventListener("click", function(event){
  document.getElementsByClassName("flipper")[0].style.transition = "0.6s";
  flip();
  userIcon.updateIcon("user-square");
  var index = gameObj.firstMove === "user" ? 0 : 1;
  document.getElementsByClassName("open-icon-select-btn")[index].style.backgroundImage = 
    "url('icons/" + userIcon.icon + "/" + userIcon.icon + " " + userIcon.colour + ".png')";
});

Also, when you reference the same element more than once, you should assign it to a variable. For example, I see the following at least twice in your code:

document.getElementsByClassName("confirm-btn")

I would assign this to a variable like below and then reference it like:

var iconConfirmBtn = document.getElementsByClassName("confirm-btn");
iconConfirmBtn[0].addEventListener('click', function() {.......});
iconConfirmBtn[1].addEventListener('click', function() {.......});

Actually, even with the above, I would figure out a way to only have a single click event handler for both buttons which would allow you to delete an entire event handler.

There are many such sections of code where 98% of the code is repeated. Make your code DRY and you will have a very impressive project to add to your portfolio.

1 Like
#4

Ah yes, I see the problem, it took me a minute to get that! I’ll get that fixed, I suspect it won’t take very long. As for the restart option after each game, I thought that may be annoying for anyone who wanted to smash out some games quickly, but I’ll give it some thought.

Thank you very much for the feedback! If I could just ask you one more question…do you think the animation that plays upon selecting who goes first is successful in indicating the function of the button? I thought just having a button that says swap was a bit vague. Thanks again!

#5

I’m definitely guilty of doing this on more than one project, I’ll get to cleaning up these monstrous looking bits of code looking better asap. Thanks for the advice on the assigning the elements a variable, I have wondered how much an element should be referenced before it becomes worthwhile to give them a variable and that is a definitive answer!

Thanks for the feedback!

#6

No problem! Glad to help.

Regarding the swap function, I do think it was a little vague, but I can suggest two simple fixes to make it easier for the player to understand what it is without playing around with it too much.

  1. Have a “You” text above the icon that you’re playing as
  2. Maybe change “Swap” to “Swap Order”
1 Like
#7

I like simple! Good ideas too, shouldn’t too hard to work in. Thanks for your time :slight_smile:

#8