Never knowing what a class should contain?

I suck at seperating data for some reason.

So I'm trying to rewrite how I'm making a SFML game to make better use of classes and OOP to make the code look cleaner. From what I've read it's best to make your own base class and derive from this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
class MySprite : public sf::Drawable
{
private:
	sf::Texture* texture;
	sf::Shape* sprite;
	sf::Color color;
	bool enabled;
public:
	MySprite(sf::Shape* s, sf::Texture* t, sf::Color c, sf::Vector2f position, sf::IntRect size) : sprite(s), texture(t) {
		sprite->setTexture(texture);
		sprite->setFillColor(color);
		sprite->setTextureRect(size);
		sprite->setOrigin(size.width / 2, size.height / 2);
		sprite->setPosition(position);
	}
	~MySprite() { delete sprite; }
	void setEnabled(bool e) { enabled = e; }
	const bool getEnabled() { return enabled;  }
	virtual void draw(sf::RenderTarget& rt, sf::RenderStates rs) { rt.draw(*sprite); }
};



A pretty simple class, but I'm having doubts to what this base class should contain.

For instance I've left the function "move" out of this base class as I don't know whether to have a MoveableSprite derivation of this base or PhysicsSprite or things such as SpriteWithAnimation .

I am really struggling to figure out what a class should have and shouldn't have. I would like to keep drawing and physics seperated so I can experiment with multi-threading.

Q) What should be in a class and what shouldn't? How many derivaitves are necessary? I don't want to make a game engine as such, but something to help me not have to write the same thing over and over again when making a topdown 2D game.


Something I wanted to clarify as well as I am far from seeing if this code is working while I work on the rest of the framework:

sf::Shape* sprite;

I am pretty sure that sf::Shape is a pure virtual just like sf::Drawable.

With my pointer to sf::Shape object I'm planning to store a new sf::CircleShape which I pressume is a child of Shape.

If I new a child class and store it in it's parent class, when I perform delete on parent class, will it release the entire memory using by the child? Or only remove what the base can point to?

I think I should clarify this before I start making derivatices as I'm not sure if my base must be a pure virtual or not when it comes to releasing things you can only point partially too (I'm not a polymorphism expert).

Thanks in advance. Any other useful tips for seperating data into classes welcome.
Last edited on
Figuring out how to build your classes is one of the hardest parts of OOP, so you aren't alone.

Looking at your class, I see that you have color and texture attributes in your class, but those appear to be attributes of class sprite also. It's generally a bad idea to keep two copies of the same information because it's too easy to forget to update them both.

So other than the sprite, the only thing left in your class is the enabled flag. Everything else in the constructor just sets attributes of the sprite. So what are you really trying to accomplish here?

Also, when using pointers, you should always decided and DOCUMENT who owns the pointer. The owner is the one responsible for deleting it. Since your destructor deletes the sprite, that means your class takes ownership of the sprite that's passed in the constructor. And since you don't set the texture member pointer or delete it, I don't know what's going on there.

So basically, it looks to me like MySprite doesn't add any value to sf::Shape.

Some other random thoughts on class design:

- Don't get class happy. Avoid the urge to make a separate class to do every little thing.

- Think carefully about whether a relationship is an "is-a" or a "has-a". That determines whether a class should derive from another class, or whether it should contain a member of the other class.

- Remember that classes are supposed to help you write your code, not get in the way of doing so. If you start to find that the classes are getting in the way of things, it's a good sign that you poorly designed classes.

- When creating methods, your goal should not be to "do the work" but rather to "make doing the work easy." For example, if you need to print the attributes of a class, don't create a method that prints to cout, create a method that prints to an ostream, and let the caller pass cout as the parameter. This will have the effect of making your code much more flexible.


> For instance I've left the function "move" out of this base class as I don't know whether to have
> a MoveableSprite derivation of this base or PhysicsSprite or things such as SpriteWithAnimation

We would have to iterate over the class hierarchy design a few times before committing on a particular interface for the base class. If this is he first attempt at writing a program for this domain, a throw-away design prototype to support iterative incremental refinement would be useful.


> I am really struggling to figure out what a class should have and shouldn't have.

In general, it is easier to add more functionality to a class when it is clear that requirement exists, than to remove functionality at a later stage when we realise that it is not needed or that it is better to provide it in some other component.


> If I new a child class and store it in it's parent class, when I perform delete on parent class,
> will it release the entire memory using by the child?

Yes, provided the vase class destructor is virtual.

Almost always, a base class should have a virtual destructor; delete on a pointer to base, where the object involved is of a derived class, engenders undefined behaviour if the destructor in the base class is not virtual.


Ideally make the class at the root of a hierarchy a pure interface: a collrction of pure virtual functions with no implementation other than explicitly defaulted foundation operations.
See: Rule of zero (rule of five defaults): http://en.cppreference.com/w/cpp/language/rule_of_three


> I would like to keep drawing and physics seperated so I can experiment with multi-threading.

Even if concurrency is not involved, keeping separate concerns separate is fundamental to robust, maintainable designs.


In the class outlined (MySprite)

Avoid raw owning pointers; consider using a smart pointer for things like this:
1
2
3
// sf::Shape* sprite;
std::unique_ptr<sf::Shape> sprite ;
// http://en.cppreference.com/w/cpp/memory/unique_ptr 


Avoid returning const-qualified values (in this case - return a scalar value - the compiler is required to ignore the const). And make the code cost correct:
1
2
// const bool getEnabled() { return enabled;  }
bool getEnabled() const { return enabled;  }
> class MySprite : public sf::Drawable
¿what's wrong with sf::Sprite?

> For instance I've left the function "move" out of this base class as I don't
> know whether to have a MoveableSprite derivation of this base or
> PhysicsSprite or things such as SpriteWithAnimation .
¿what's a physics_sprite?
¿are you putting the game logic onto the graphics components?

> If I new a child class and store it in it's parent class, when I perform
> delete on parent class, will it release the entire memory using by the child?
> Or only remove what the base can point to?
not sure if follow
1
2
3
sf::Shape *sprite;
sprite = new sf::CircleShape;
delete sprite; //¿what destructors are called? 
the answer there would be: it depends.
if the base destructor was declared virtual, then it would call the derived destructor.
Thanks for the answers guys I'll reply in turn -

@DHayden

Looking at your class, I see that you have color and texture attributes in your class, but those appear to be attributes of class sprite also. It's generally a bad idea to keep two copies of the same information because it's too easy to forget to update them both.


The texture and color objects relative to the sprite are local to the sprite, so my idea was to load textures in AssetManager class then pass the textures through pointers to my own objects:

 
MySprite sprite(new sf::CircleShape,AssetManager.GetTexture(some_texture_enum),...


So other than the sprite, the only thing left in your class is the enabled flag. Everything else in the constructor just sets attributes of the sprite. So what are you really trying to accomplish here?


I'm not exactly sure what I'm trying to accomplish to be brutally honest. I have some working code which is working great, but it was very tedious trying to construct more complicated objects from shapes.

Here's a snippet - https://pastebin.com/j7ZZsGiY

As you can see I have classes Armor and Turret which can be "equipped" to the Robot class. I'm trying to find a better solution with polymorphic classes. It would be nice creating an interface where I can produce graphics on the screen and easily "group" things together so they move as a whole.

Also it seemed like polymorphic classes would be a great way to store "RobotParts" such as chassis, or weapon or circuitry if they can be stored in the same vector:

1
2
3
4
5
Base* body = new DerivedChassis();
Base* weapon = new DerivedWeapon();
std::vector<Base*> robot;
robot.push_back(body);
robot.push_back(weapon);


This is probably not the best way to go, but it seems polymorphism is crucial for game development so I'm just trying things out as I go along haha.



- Don't get class happy. Avoid the urge to make a separate class to do every little thing.

- Think carefully about whether a relationship is an "is-a" or a "has-a". That determines whether a class should derive from another class, or whether it should contain a member of the other class.


These are also very good points. I might not go as far to make several sprite classes but maybe a tile class and a moveable object/character/npc class.

As for the is-a and has-a, this is the thing that's grinding me down the most.

I can picture in my head what I need my Robot to do: equip stuff(stuff being the keyword here; weapons, armor, new chips, colour schemes etc), move, shoot whatever weapon it has, collide with other robots or the arena. Seems simple in theory, putting it into SFML is not so easy.


I think the best thing I could do for advice is to ask you to google "Bot Arena 3". It's a game I used to play in IT when I should have been doing my school work. He never did make a 4th so I'm taking it up myself. :-)

The only difference being my robots are a collection of shapes instead of fancy graphics.
Last edited on
@JLBorges


We would have to iterate over the class hierarchy design a few times before committing on a particular interface for the base class. If this is he first attempt at writing a program for this domain, a throw-away design prototype to support iterative incremental refinement would be useful.


This is my first attempt at creating the game. I've been dinkying about with SFML and I think I understand it enough to be able to make a simple game. I probably shouldn't try to make such a framework on my first attempt at making a game, but I'll have a go at anything. I will have to research what a throw-away design prototype and get back to you on what should be recommended if a rewrite is necessary.


Even if concurrency is not involved, keeping separate concerns separate is fundamental to robust, maintainable designs.


I can be honest here and say this is something I can't do well. OOP is not my strong point. Although I can code and get things to work the way I want them, I always end up with strange dependancies and not being able to keep apples and pears seperate even though I know they have nothing to do with each other. It's like my code always needs to know what everything else is doing, which I think is very poor design. Need to get my stuff doing one thing and doing it well.


Avoid raw owning pointers; consider using a smart pointer for things like this:


I've still not dabbled much into the memory header, and I'm still not totally satisfied I can use standard pointers extensivley so I've been sticking to using raw pointers to force proper procedure's of newing and delete.

Somewhat of a good job too otherwise I wouldn't have known to make Base destructor virtual for pointer deletion :-)

BTW How does the virtual destructor go for smart pointers, same principles?


Thanks for your reply, great here from you again and a huge help as usual, Thank you.



@NE555


1 - ¿what's wrong with sf::Sprite?
2 - ¿what's a physics_sprite?
3 - ¿are you putting the game logic onto the graphics components?


1) sf::Sprites can only be rectangular textures - https://www.sfml-dev.org/documentation/2.0/classsf_1_1Sprite.php

I can not pass it a shape. Shapes are the only way I can use graphics and animate it. My Robots are a collection of shapes. Bullets will most likely be small circles and I can have my own animations such as a group of circles circuling the player, or a "shield". The graphics are only going to be primitive. So I thought using sf::Drawable would be the one to derive from as I'm not using spritesheets. I could be wrong I don't know can't wait to here more if I am.

2) A PhysicsSprite would be a sprite that is updated by the PhysicsEngine, a PhysicsSprite would be processed differently to say TileSprite which does not move and renders every frame...?

I don't know haha. I would image not all sprites are the same or processed the same way.

3) I'm really trying not to, I'd like everything to be as modular as possible. If you have any ideas how I can keep graphics and functionality seperate I'd be happy to read!


Anyway thanks guys for taking the time. Appreciate it.

Also let me know if you want my full code if you think you could do something with it to make it better.
Last edited on
perhaps you could alpha mask the sprite to make it look non-rectangular.
> I will have to research what a throw-away design prototype (is)

Stroustrup in 'Programming: Principles and Practice Using C++'
Build a small, limited version of the program that solves a key part of the problem. When we start, we rarely know the problem well. We often think we do (...), but we don’t. Only a combination of thinking about the problem (analysis) and experimentation (design and implementation) gives us the solid understanding that we need to write a good program.

So, we build a small, limited version
• To bring out problems in our understanding, ideas, and tools.
• To see if details of the problem statement need changing to make the problem manageable.

It is rare to find that we had anticipated everything when we analyzed the problem and made the initial design. We should take advantage of the feedback that writing code and testing give us.

Sometimes, such a limited initial version aimed at experimentation is called a prototype . If (as is likely) our first version doesn’t work or is so ugly and awkward that we don’t want to work with it, we throw it away and make another limited version based on our experience. Repeat until we find a version that we are happy with. Do not proceed with a mess; messes just grow with time.



> It's like my code always needs to know what everything else is doing

For keeping the drawing and physics separate, one possibility is to encapsulate the physics logic in an abstract strategy interface. http://www.oodesign.com/strategy-pattern.html


> How does the virtual destructor go for smart pointers, same principles?

Yes. For std::unique_ptr<> for a base class, the destructor must be virtual.
std::shared_ptr<> is more intelligent.
Nevertheless, it is a good idea to always default to declaring the base class destructor as virtual.
Topic archived. No new replies allowed.