SFML community forums
General => SFML projects => Topic started by: KrySoar on May 19, 2022, 03:11:44 pm
-
I'm going back into programming after several years, so I decided to make a simple snake game to begin, could you give me some advice on my way of programming please ?
The code is here : https://github.com/KrySoar/SnakeGame
-
Welcome back to the land of programming! :)
Here are some comments:
- Using operator overload for the Snake is kind of neat, but also quite misleading and not very readable - you have to know what snake++ really does. Instead you might just want to implement a method with a name, that makes the functionality clear.
- You're mixing tabs and spaces, might want to decide for one :D
- You call new BodyPart() but never call delete. I recommend to not use manual memory management at all. In the situation of the body parts, you could just go with a std::vector<BodyPart> and don't put the head into the vector
- You could derive the Snake from sf::Drawable and implement the draw method, so you can write window.draw(snake) instead
- You should use the <random> header instead of rand(), then again for such a small thing it might be okay
- One potential "improvement" is to pass a direction vector instead of letters for the move direction. Maybe you can declare some constants for Up, Down, Left, Right
-
Welcome back to the land of programming! :)
Here are some comments:
- Using operator overload for the Snake is kind of neat, but also quite misleading and not very readable - you have to know what snake++ really does. Instead you might just want to implement a method with a name, that makes the functionality clear.
- You're mixing tabs and spaces, might want to decide for one :D
- You call new BodyPart() but never call delete. I recommend to not use manual memory management at all. In the situation of the body parts, you could just go with a std::vector<BodyPart> and don't put the head into the vector
- You could derive the Snake from sf::Drawable and implement the draw method, so you can write window.draw(snake) instead
- You should use the <random> header instead of rand(), then again for such a small thing it might be okay
- One potential "improvement" is to pass a direction vector instead of letters for the move direction. Maybe you can declare some constants for Up, Down, Left, Right
Thank you so much for you review,
-I changed the operator override to a function named grow(),
- and for the spaces mixed with tabs I think it's only on some lines (I'm new to vim so maybe I did something wrong).
-For the vector<BodyPart*> I thought that I needed to do that to later modify the bodypart but I changed it to vector<BodyPart> and it works.
-Yes for the random I know that this is deprecated but I wanted something simple and not very very random
-and for the direction I made an enum in constvars.hpp.
-For the next project I will derive some classes from sf::Drawable or sf::RectangleShape (I didn't think of that but this is so much better you are right) thank you for the time you took for me !
-
Cool that you directly applied my feedback! :)
Three more things I just saw after checking the code again
- for(BodyPart bp : snakeBody) this should be a BodyPart& otherwise, you're constantly creating a copy, but instead you want a reference
- Instead of snakeBody.push_back(snakeHead); you could snakeBody.emplace_back(snakeHead);. It does the same thing basically, but it uses the move constructor to move the snakeHead into the vector.
- If you don't use sf::Drawable, then I recommend to change the draw signature to take a reference to the window, instead of a raw pointer: draw(sf::RenderWindow& win)
-
Cool that you directly applied my feedback! :)
Three more things I just saw after checking the code again
- for(BodyPart bp : snakeBody) this should be a BodyPart& otherwise, you're constantly creating a copy, but instead you want a reference
- Instead of snakeBody.push_back(snakeHead); you could snakeBody.emplace_back(snakeHead);. It does the same thing basically, but it uses the move constructor to move the snakeHead into the vector.
- If you don't use sf::Drawable, then I recommend to change the draw signature to take a reference to the window, instead of a raw pointer: draw(sf::RenderWindow& win)
Thank you !