I want to optimize my code

[Mariusmssj]

In Paul's words:"Learn about virtual functions". If you don't understand, it means you don't have a correct understanding about virtual inheritance. If you learn about them, it will make sense.

Sorry for the kinda harsh answer, but sometimes you need to stop to understand before rushing too far.

oh on it's not harsh at all xD i am reading about them

[Mariusmssj]

after doing some reading i understand move is a default function, for me it's just random movement BUT in zombie, man and hunter the move will be specific/unique to them.


this means that i will have to define :

virtual bool IsHumanInRange(Body &);
virtual void Hunt(Body &);

in the body to but change in the hunter and badger

i hope i am right xD

[monarch_dodra]

after doing some reading i understand move is a default function, for me it's just random movement BUT in zombie, man and hunter the move will be specific/unique to them.


this means that i will have to define :

virtual bool IsHumanInRange(Body &);
virtual void Hunt(Body &);

in the body to but change in the hunter and badger

i hope i am right xD

That sounds right

[Mariusmssj]

ok i tried to search and read but i am still bit confused of how to Populate aEnemies with pointers to ALL enemies "/

	int zomb = 5;
	int hol = 5;
	int hunt = 2;
	int range = 20;

	//creates the objects in the game
	Man a_man;
	std::vector<Zombie> zombies(zomb);
	std::vector<Hole> holes(hol);
	std::vector<Hunter> hunters(hunt);

	std::vector<Body*> aEnemies; //Populate this with pointers to ALL enemies, regardless of type.
	//This will be your main object
	std::vector<Body*>::iterator it = aEnemies.begin();
	std::vector<Body*>::iterator itEnd = aEnemies.end();
	for( ; it!=itEnd ; ++it )
	{
    	if (it->isHumanInRange(a_man))
    	{
        	it->hunt(a_man);
    	}
    	else
    	{
        	it->move();
    	}
	}

Nut now i understand what it will do I will have a vector FULL of enemies - zombies, hunters badgers and such, and i will have this loop, which will have 2 things, MOVE and HUNT but what is cool that move and hunt will be specific. The move for hunter and zombie will be different, kinda like a overloaded function. Am i right?

[monarch_dodra]

ok i tried to search and read but i am still bit confused of how to Populate aEnemies with pointers to ALL enemies "/

    int zomb = 5;
    int hol = 5;
    int hunt = 2;
    int range = 20;

    //creates the objects in the game
    Man a_man;
    std::vector<Zombie> zombies(zomb);
    std::vector<Hole> holes(hol);
    std::vector<Hunter> hunters(hunt);

    std::vector<Body*> aEnemies; //Populate this with pointers to ALL enemies, regardless of type.
    //This will be your main object
    std::vector<Body*>::iterator it = aEnemies.begin();
    std::vector<Body*>::iterator itEnd = aEnemies.end();
    for( ; it!=itEnd ; ++it )
    {
        if (it->isHumanInRange(a_man))
        {
            it->hunt(a_man);
        }
        else
        {
            it->move();
        }
    }

Nut now i understand what it will do I will have a vector FULL of enemies - zombies, hunters badgers and such, and i will have this loop, which will have 2 things, MOVE and HUNT but what is cool that move and hunt will be specific. The move for hunter and zombie will be different, kinda like a overloaded function. Am i right?

yeah, the result is kind of the same as an overload, but conceptually, given it's run time dynamic behavior, it is also completely different.

Do you understand why you have a vector of pointers to enemies, rather than actual enemies? There are multiple reasons, which involve slicing, static typing, and the fact that virtual calls are only resolved when called through pointers (->) (well, it also works with references, but this is less recommended, from a maintenance point of view).

Usually, you do something like this:

body* bodyHunter = new hunter

However, this is rife with potential memory leaks. It is easier to create an auto object, and point to that:

{
    hunter actualHunter
    body* bodyHunter = &actualHunter
    //actualHunter is destroyed here
}

This is why you have a vector of Badgers/Zombies etc. It also has the advantage of less calls to new.

This is how you populate it, I guess.

    std::vector<Zombie> zombies(zomb);

    std::vector<Body*> aEnemies;

    std::vector<Zombie>::iterator it = zombies.begin();
    std::vector<Zombie>::iterator itEnd = zombies.end();
    {
        aEnemies.push_back(&*it);
    }
... ditto with Hunters etc.

&*it means "the address of where it points" This is usually not needed, but sometimes the type of it is not an actual pointer. For example, you always have to do this for lists. With vectors, you can usually get away with using it straight up.

(I'm sure there are efficient std algorithms to do this, but that's besides the point)

On a side note, in the above code I had written, the correct code is:

    std::vector<Body*>::iterator it = aEnemies.begin();
    std::vector<Body*>::iterator itEnd = aEnemies.end();
    for( ; it!=itEnd ; ++it )
    {
        if ((*it)->isHumanInRange(a_man))
        {
            (*it)->hunt(a_man);
        }
        else
        {
            (*it)->move();
        }
    }

This is because it is an iterator to a pointer, not an iterator to an object.

[Mariusmssj]

yeah, the result is kind of the same as an overload, but conceptually, given it's run time dynamic behavior, it is also completely different.

Do you understand why you have a vector of pointers to enemies, rather than actual enemies? There are multiple reasons, which involve slicing, static typing, and the fact that virtual calls are only resolved when called through pointers (->) (well, it also works with references, but this is less recommended, from a maintenance point of view).

Usually, you do something like this:

body* bodyHunter = new hunter

However, this is rife with potential memory leaks. It is easier to create an auto object, and point to that:

{
    hunter actualHunter
    body* bodyHunter = &actualHunter
    //actualHunter is destroyed here
}

This is why you have a vector of Badgers/Zombies etc. It also has the advantage of less calls to new.

This is how you populate it, I guess.

    std::vector<Zombie> zombies(zomb);

    std::vector<Body*> aEnemies;

    std::vector<Zombie>::iterator it = zombies.begin();
    std::vector<Zombie>::iterator itEnd = zombies.end();
    {
        aEnemies.push_back(&*it);
    }
... ditto with Hunters etc.

&*it means "the address of where it points" This is usually not needed, but sometimes the type of it is not an actual pointer. For example, you always have to do this for lists. With vectors, you can usually get away with using it straight up.

(I'm sure there are efficient std algorithms to do this, but that's besides the point)

On a side note, in the above code I had written, the correct code is:

    std::vector<Body*>::iterator it = aEnemies.begin();
    std::vector<Body*>::iterator itEnd = aEnemies.end();
    for( ; it!=itEnd ; ++it )
    {
        if ((*it)->isHumanInRange(a_man))
        {
            (*it)->hunt(a_man);
        }
        else
        {
            (*it)->move();
        }
    }

This is because it is an iterator to a pointer, not an iterator to an object.

well all the pointers are so you could have all the list of all the enemies and just using that, you could move the specific ones.

and

    std::vector<Zombie> zombies(zomb);

    std::vector<Body*> aEnemies;

    std::vector<Zombie>::iterator it = zombies.begin();
    std::vector<Zombie>::iterator itEnd = zombies.end();
    {
        aEnemies.push_back(&*it);
    }
... ditto with Hunters etc.

i need a loop here don't i?

[monarch_dodra]

I did not understand either question

[Mariusmssj]

"Do you understand why you have a vector of pointers to enemies, rather than actual enemies?"

yes i so, because this gives me kinda a list of all the objects which can attack the man. So i could control movement of zombie, hunter and ect. with that one for loop

and

[nuzzle]

The purpose of OO is to make future changes easy but I feel the use of the Body superclass rather is convoluting this design now. It's being used for too much. I think it should be limited to handle position and movement only.

The only property that is unambigiously shared between zombies, hunters, enemies, badgers and whathaveyou in this design is that they have a position and can move. Other relationships are much better handled outside of Body. In short subclasses of Body shouldn't be assumed in Body.

[Mariusmssj]

The purpose of OO is to make future changes easy but I feel the use of the Body superclass rather is convoluting this design now. It's being used for too much. I think it should be limited to handle position and movement only.

The only property that is unambigiously shared between zombies, hunters, enemies, badgers and whathaveyou in this design is that they have a position and can move. Other relationships are much better handled outside of Body. In short subclasses of Body shouldn't be assumed in Body.

so i should re-work it using SRP?