I've been hard at work this week programming breakout again, but this time I'm putting heavy emphasis on programming using object oriented techniques. So, I am having my Block/Ball/Paddle class all handle their Variables, while I have a GameLogic class that handles all the logic(Collision Detection/Updating the other classes/Etc) and a GameDraw class which Draws all my objects(Just draws them to the screen). Then I have a MainGame class which has an instance of GameLogic and GameDraw, an instance of the other classes I need, and it's responsibility is to run the game. I spent a lot of time Pre-Visualizing how I was going to set up my classes, and what their responsibility's would be. This project abides by the single responsibility principle, and the Open-Closed principle(I hope, I really have a weak understanding of it).
Here's the link to the .zip fie containing my project:
http://www.mediafire.com/?r7l6hfxf8dh8c37
What Do You Guys Think of How I Programmed This?
Author
And I didn't link the code! FacePalm!
I'll have that up really soon!
Here's the link to the .zip file containing my project:
http://www.mediafire.com/?r7l6hfxf8dh8c37
I'll have that up really soon!
Here's the link to the .zip file containing my project:
http://www.mediafire.com/?r7l6hfxf8dh8c37
First off, if you're going to share your code, don't include things like the contents of the Debug/ folder, the intellisense cache or other extraneous compiler generated files. Otherwise, there doesn't seem to be enough completed to comment on the overall design. There are a number of implementation details that seem non-kosher, however. From a quick glance through: you seem to have some vectors that you never resize, and it seems like you don't intend on resizing them. In that case you should consider using std::array instead of std::vector. Also you seem to use a lot of non-const references when you only need const references (e.g.: Block and Ball's constructors). You also have a number of functions that return copies of objects rather than references to objects. Ex: Block::GetImage() could probably be changed to returning a const reference. There also seems to be a quite a bit of redundant computation going on. For instance, why store ball position as a member variable if you can get it from the sprite object?
Also, GameMain has protected member variables. If a class has non-public variables that implies that the class has a class invariant. With private variables only the members of that class is responsible for maintaining the class invariants. However, if you have protected member variables then every derived class is also responsible for maintaining the class invariants. This is generally a lot more trouble than it's worth.
Also, GameMain has protected member variables. If a class has non-public variables that implies that the class has a class invariant. With private variables only the members of that class is responsible for maintaining the class invariants. However, if you have protected member variables then every derived class is also responsible for maintaining the class invariants. This is generally a lot more trouble than it's worth.
Author
What you said about GameMain I removed. For the redundant computation: Later on in my code, in GameLogic/GameDraw especially, I have some code that would be almost impossible to read unless I have those variables. They're to help my program make more sense later on. Also, GetImage() is never used, but that's a good idea. This is stuff I'll have to remember later on.
For the redundant computation: Later on in my code, in GameLogic/GameDraw especially, I have some code that would be almost impossible to read unless I have those variables.
You can make functions with the same names that grabs the values as needed which will give you equivalent readability. See also Don't Repeat Yourself (DRY).
Also, GetImage() is never used, but that's a good idea.[/quote]
You might want to get out of the habit of writing functions that are never used. See You Aren't Going to Need It (YAGNI).
GameLogic::CheckCollision() -- this if is freaking terrible. After 10 minutes reading I don't get it.
Why do you every time repeat this GetIsHit()? And also this freaking LogicBall->GetCollision().Intersects(LogicBoard[index]->GetTop())... It would be nice to refactore it to
I hope I've gave you some ideas about how to improve your code.
And also I suggest you to use pointers instead of references. It's a little bit flexible.
Why do you every time repeat this GetIsHit()? And also this freaking LogicBall->GetCollision().Intersects(LogicBoard[index]->GetTop())... It would be nice to refactore it to
void GameLogic::CheckCollision()
{
...
if (logic_ball->GetCollisionMgr()->Intersects(logic_board[index]) && LogicBoard[index]->GetIsHit() == false) {
...
logic_ball->UpdateVelocity(); // this func knows that GetCollisionMgr()->Intersects(...) have cached info about intersection and know how to apply velocity
}
...
I hope I've gave you some ideas about how to improve your code.
And also I suggest you to use pointers instead of references. It's a little bit flexible.
Author
Your interface Manager idea is smart, and I might implement it later. Thank you for your input, just please be less harsh on people in the beginners section.
I dislike how you called my code "freaking terrible", I'm posting in the beginners section and your post made me get Effective C++, Code Complete, and C++ Primer Plus.
I dislike how you called my code "freaking terrible", I'm posting in the beginners section and your post made me get Effective C++, Code Complete, and C++ Primer Plus.
Author
This code is officially dead, I'm going to try something simpler before attempting breakout again. My interface is awful (As the guy above pointed out), and I didn't plan my code at all. I'm going to try to become a better programmer by taking a smaller step, because I bit off more than I could chew.
superman3275 You can use sites like http://www.codeplex.com/ to post your code, it's much easier for us to access.
I didn't look over all the code, but AlexB.hpp it would appear he called the GetIsHit() each time so it doesn't check a block that has been hit? I would suggest once you hit a "block" you end the for loop. Does index refer to the 19 blocks you can hit? Sorry if I'm reading the code wrong.
Also, using references is perfectly fine! I personally prefer to pass by reference.
superman3275 don't be so hard on yourself, I learned how to program from trial and error. If it wasn't' for the thousands of times I had to reprogram the same code, I wouldn't even be able to program half the stuff I do today. I do however recommend working by setting small goals to reach your big goals. I'm doing this on my RPG project posted in my Journal, and it's coming along perfectly.
I didn't look over all the code, but AlexB.hpp it would appear he called the GetIsHit() each time so it doesn't check a block that has been hit? I would suggest once you hit a "block" you end the for loop. Does index refer to the 19 blocks you can hit? Sorry if I'm reading the code wrong.
Also, using references is perfectly fine! I personally prefer to pass by reference.
superman3275 don't be so hard on yourself, I learned how to program from trial and error. If it wasn't' for the thousands of times I had to reprogram the same code, I wouldn't even be able to program half the stuff I do today. I do however recommend working by setting small goals to reach your big goals. I'm doing this on my RPG project posted in my Journal, and it's coming along perfectly.
Author
That's smart! Black-Rook, the GetIsHit() function is exactly for that. IsHit makes it so that I don't draw blocks that have been hit and I don't check for collision on them either. I read a post about how you should set small goals and focus on getting stuff up on the screen to motivate yourself, and I think that sounds smart. As lazyfoo.net said, I'm entering the realm of pre visualization, where I have to actually think about my code before programming!?!?! It's unheard of. Thanks for your input 
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement