Hello! i have been programming in C++ for about a year now. I was just wondering if some of you veteran programmers could give me your thoughts on my code, let me know what i have done wrong, what ive done well, ect. don't hesitate to say something in here is absolutely horrible, please let me know. there is a few parts in here that i hate the way i did, but wasn't sure about another way of doing it. and it just being a prototype some of the things i just didn't bother messing with.

one thing im going to add before moving switching back to using OOP, is timing. if you guys have any advice on that, it would be appreciated as well.

im interested in what you think about how i handled the states, if i should go about it another way, or if the way i did it is good.

Here is a link to paste-bin if you don't want to download the .zip attached below. http://pastebin.com/bmCmwYRA

this is using the SFML 2.1 Lib, in case anyone is looking at this is interested in SFML/Seeking Help with SFML.

Thanks!

For a start you could avoid including a huge amount of headers just in case you need them a year later maybe, split up those 500 line functions and not waste 40 characters on an average line cause of overly deep indentation that messes up viewing the code and split your code into more files.

void StringChangeDif(sf::Text & t1, int & a)
{
                switch(a)
                {
                case 0:
                                t1.setString("Cake");
                                break;
                case 1:
                                t1.setString("Easy");
                                break;
                case 2:
                                t1.setString("Medium");
                                break;
                case 3:
                                t1.setString("Hard");
                                break;
                case 4:
                                t1.setString("Impossible");
                                break;
                }
}

I suggest doing this instead:

void StringChangeDif(sf::Text & t1, int a) // No need to pass primitive types by reference if you are not going to modify them
{
  static std::string const difficulty_name[5] = {"Cake", "Easy", "Medium", "Hard", "Impossible"};
  
  t1.setString(difficulty_name[a]);
}

Similarly for most `switch' statements in your code and some `if-else if' sections. Whenever you have repetitive code like that, you should probably try to use a data structure instead.

I don't like the switch or Alvaro's code ;)

The switch is better behaved since it wont do anything if passed a value not in the range 0 to 4. I'd put in a default case that asserts or something though. All switch statements should have a default case!

Alvaro's code doesn't do any range checking at all :(

For a start you could avoid including a huge amount of headers just in case you need them a year later maybe, split up those 500 line functions and not waste 40 characters on an average line cause of overly deep indentation that messes up viewing the code and split your code into more files.

the headers aren't really an issue as this is a prototype. once i get the hang of everything im trying this project ends. will take the note in the future for those OOP focused projects. formatting wont be an issue in the future, again, this being a dinky prototype im not going to spend too much time making it look pretty,i can navigate around it well enough. the indentation looks the best IMO and works with my widescreen monitors, unfortunately on pastebin it is not formatted the same way, and depending on your IDE and formatting rules i suppose it would also look different. this being a prototype i didn't want to use classes, im still trying to learn how to handle certain game logic, and using classes interferes with the work that really needs to get done.

thanks for your input.

Alvaro's code doesn't do any range checking at all

Ah, we have a philosophical difference of opinion here. I don't think the code should be responsible for doing something reasonable if you use it wrong. You may not like my philosophy, but the whole language is built that way.

I would normally write asserts in a function like that, but I didn't want to complicate things.

void StringChangeDif(sf::Text & t1, int & a)
{
                switch(a)
                {
                case 0:
                                t1.setString("Cake");
                                break;
                case 1:
                                t1.setString("Easy");
                                break;
                case 2:
                                t1.setString("Medium");
                                break;
                case 3:
                                t1.setString("Hard");
                                break;
                case 4:
                                t1.setString("Impossible");
                                break;
                }
}

I suggest doing this instead:

void StringChangeDif(sf::Text & t1, int a) // No need to pass primitive types by reference if you are not going to modify them
{
  static std::string const difficulty_name[5] = {"Cake", "Easy", "Medium", "Hard", "Impossible"};
  
  t1.setString(difficulty_name[a]);
}

Similarly for most `switch' statements in your code and some `if-else if' sections. Whenever you have repetitive code like that, you should probably try to use a data structure instead.

wonderful! i knew there had to be a better way of doing that. that was the main thing that aggravated me in my code was those absurdly long switch's/ifs. Thanks!

For a start you could avoid including a huge amount of headers just in case you need them a year later maybe, split up those 500 line functions and not waste 40 characters on an average line cause of overly deep indentation that messes up viewing the code and split your code into more files.

the headers aren't really an issue as this is a prototype. once i get the hang of everything im trying this project ends. will take the note in the future for those OOP focused projects. formatting wont be an issue in the future, again, this being a dinky prototype im not going to spend too much time making it look pretty,i can navigate around it well enough. the indentation looks the best IMO and works with my widescreen monitors, unfortunately on pastebin it is not formatted the same way, and depending on your IDE and formatting rules i suppose it would also look different. this being a prototype i didn't want to use classes, im still trying to learn how to handle certain game logic, and using classes interferes with the work that really needs to get done.
thanks for your input.

Since this is a learning project, you should try to make the code as good as possible, use classes (if they make sense), break down large functions into smaller functions (if you can give them good names), avoid using global variables, etc. If you allow yourself bad habits in a tiny program where it wouldn't be that much work to do things right, heaven knows what you will do in a large project where doing things right sometimes requires huge efforts.

I don't like the switch or Alvaro's code ;)

The switch is better behaved since it wont do anything if passed a value not in the range 0 to 4. I'd put in a default case that asserts or something though. All switch statements should have a default case!

Alvaro's code doesn't do any range checking at all

i think he was just trying to get the point across, not write my code for me. i prefer his way of doing it just because its less tedious repetitive crap to do :)

For a start you could avoid including a huge amount of headers just in case you need them a year later maybe, split up those 500 line functions and not waste 40 characters on an average line cause of overly deep indentation that messes up viewing the code and split your code into more files.

the headers aren't really an issue as this is a prototype. once i get the hang of everything im trying this project ends. will take the note in the future for those OOP focused projects. formatting wont be an issue in the future, again, this being a dinky prototype im not going to spend too much time making it look pretty,i can navigate around it well enough. the indentation looks the best IMO and works with my widescreen monitors, unfortunately on pastebin it is not formatted the same way, and depending on your IDE and formatting rules i suppose it would also look different. this being a prototype i didn't want to use classes, im still trying to learn how to handle certain game logic, and using classes interferes with the work that really needs to get done.
thanks for your input.

Since this is a learning project, you should try to make the code as good as possible, use classes (if they make sense), break down large functions into smaller functions (if you can give them good names), avoid using global variables, etc. If you allow yourself bad habits in a tiny program where it wouldn't be that much work to do things right, heaven knows what you will do in a large project where doing things right sometimes requires huge efforts.

I completely get that and agree 100%. im working on making my functions better, and ive come a LONG way, still got quite a bit left to learn though. This is a learning project in the sense that, i wanted to learn how to implement basic game fundamentals. i more than know the basics of C++ and SFML, and finished pong using classes and OOP, sprites ect. but it was lacking some of the things i wanted, (States, custom methods to make certain things simpler/better/faster, timing(which i didn't get to in the code you guys are looking at), im really OCD, so formatting is always on my mind, and this project probably looks vomit worthy outside of my IDE lol, but the way i have it set up works for what it is. the bad habits thing is definitely something i don't want to start/hang on too, i try not to use global's often if ever, and in this project the only ones i use are Width/Height, and the #includes. its going to take quite a bit of practice before id be able to keep a large project understandable. the variables i use in this that are super vague, i only do in projects where i don't expand into using classes. i gave up on trying to keep things short, and decided id rather know what something does rather than type X letters less, which im glad i started doing :).