Jump to content
  • Advertisement
Sign in to follow this  
margarit

TicTacToe game attempt review

This topic is 553 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

First of all I want to say hello to everyone! I'm new around here and this is my first topic.

Not long ago I've started to learn about games programming with C++ and I attempted to implement a CLI tictactoe game from scratch. This is my first attempt ever to program a game, but I want to learn as much as I can... because I simply enjoy doing this stuff. It's really fun.

Anyways, I do not have any firends that have experience with programming games (or C++, for that matter) and that's why I ask you guys here if you could take a look over my code and tell me what you think, it'll help me a lot.

Here's my gitlab repository.

https://gitlab.com/bogdan.margarit/TicTacToe

Thanks in advance! :)

Share this post


Link to post
Share on other sites
Advertisement

It's still asking for login.

I've logged with Bitbucket identity, but your project's page seems empty. The only activity I see is "Bogdan Margarit created project Bogdan Margarit / TicTacToe  4 days ago".

No repository, no commits or files at all.

Share this post


Link to post
Share on other sites

Oh well, it turns out that gitlab's privacy settings confused me a lil' bit. It should be OK now, I set the project to public and anyone can clone it without needing authentication. 

Share this post


Link to post
Share on other sites

Oh well, it turns out that gitlab's privacy settings confused me a lil' bit. It should be OK now, I set the project to public and anyone can clone it without needing authentication.
Nope:
$ git clone git@gitlab.com:bogdan.margarit/TicTacToe.git

Cloning into 'TicTacToe'...
The authenticity of host 'gitlab.com (52.167.219.168)' can't be established.
ECDSA key fingerprint is SHA256:HbW3g8zUjNSksFbqTiUWPWg2Bq1x8xdGUrliXFzSnUw.
ECDSA key fingerprint is MD5:f1:d0:fb:46:73:7a:70:92:5a:ab:5d:ef:43:e2:1c:35.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added 'gitlab.com,52.167.219.168' (ECDSA) to the list of known hosts.
Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

You made sure there is a repository and there are files in it? (vstrakh reported not finding a repository)

Maybe you still need to push your files or so?

Share this post


Link to post
Share on other sites

I'm sure the files are in there. I set the project's privacy so anyone could view and clone it. Not sure why it's not working.

Anyways, I've uploaded the files to uploadfiles.io. Here's the link:

https://ufile.io/a6863

Thank you for the patience. :)

Share this post


Link to post
Share on other sites
Clearly there was some care put into this because of how clean the code is...but AutoPlayer.h was not included in the archive. You need to slow down and double check yourself that you're releasing a complete presentation or people may decide not to take you seriously. 
With that said, AutoPlayer.h was basically unnecessary with a minor modification. You show an interesting usage of string construction toward the end of Board::print. That line didn't compile in VisualStudio2013 without conversion to c_str but still I thought that was 'kinda neat. All and all, pretty solid out of the gate. 

Something else, when the game is over, reset to a play again state. When the console just exits like you do, I'm not inclined to play again.

There are those that would argue that the console is not the place for games and well...I'll agree...but if you're going to abuse something, lol...take it to the extreme. [ Here ] is a personal example of using the console for what it wasn't meant for. <giggle>

Nice Work.

Share this post


Link to post
Share on other sites

I agree with Mark Kughler, it is very good code.

Some minor niggles to consider:

 

1. void Game::analyze()

It's a long function, with several blocks where you perform some check. There is however no clue what check you're doing in each block. In a year, you'll have to reverse-engineer the blocks to understand what they do.

 

You can avoid that in a few ways. One option is to move a check in a separate function, so you can attach a name to the block, like "bool Game::is_board_filled()" which immediately explains that the code is doing.

Another option is to add a line of comment above each block:

/* Check 1: Is there any space left at the board? */

This makes you can make high-level jumps from one such comment line to the next, making it easier to find some case that doesn't work, for example.

 

 

2. Your editor can't make up its mind whether to use spaces or tabs for indenting:

[attachment=35277:mixed-spaces-tabs.png]

This is how it looks in my editor. I have tabstops at multiples of 8 positions, and it's jumping back and forth. (The dark-grey >----- is a tab character.) Your code has it too, but it is less noticeable there.

 

3. using namespace std;   (in various files)

This is not universally accepted as a good idea, for two reasons. "std" is very big, so you are running the risk that you use a seemingly harmless name, and then the compiler fails and gives a weird error, because it was also used in the std:: name space.

Another reason for not recommending global import of std is that "string s;" looks like a local string type, while "std::string s;" is clearly not in your source code. In other words, the fact that you have to write a prefix "std::" makes more clear where things are coming from. You use only one namespace, so the search isn't that complicated, but imagine you import 15 name spaces in this way. Now find me this "f" function I have.... a prefixed name like mathlib::f   resolves that search in about half a second.

 

 

All suggestions are mostly "next level" ideas for you rather than things that are wrong.

Good work!

Edited by Alberth

Share this post


Link to post
Share on other sites

Thank you for the feedback! 

@Mark 

AutoPlayer was supposed to be the bot, but I thought it'd be easier to implement the code for the bot in the Player class itself, because a bot is as well a Player, right? :) 

I downloaded your code and read it and already learnd some new syntax (I'm new to C++), so thanks for that. I'll try to compile it tomorrow (got to set up visual studio - currently I'm on notepad++ and cli g++) and then fool around with it, change few lines here and there. Hope you don't mind.

@Alberth

Coming from PHP and Java, I was thinking that "using namespace" is some sort of equivalent of Java's import. I see now that it is more like import static.

About the long method, indeed it needs refactoring. To be fixed in the next "release".  :lol:

Thanks again, now I go and try to make the analyze method smarter. 

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!