CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 1 of 2 12 LastLast
Results 1 to 15 of 25
  1. #1
    Join Date
    Nov 2009
    Location
    UK
    Posts
    166

    I want to optimize my code

    Hello guys if you remember few days ago i was going crazy with trying to make my game work. Well the game works and it's all good, but i am really unimpressed with how the code looks and is managed.

    i got 6 classes in total in order to make the code better but my main still looks like a mess. I went over it few times, changed few bits and added comments.

    Could anyone see where it would be improved?

    thank you very much

    Code:
    #include<iostream>
    #include<time.h>
    #include<stdlib.h>
    #include"RandomNumber.h"
    #include"Man.h"
    #include"console.h"
    #include<windows.h>
    #include"Zombie.h"
    #include"Hole.h"
    #include<vector>
    #include<algorithm>
    
    int main()
    {
    	srand((unsigned)time( NULL ));
    
    	//allows to pick amount of holes and zombies
    	int zomb = 5;
    	int hol = 5;
    
    	//creates the objects in the game
    	Man a_man;
    	std::vector<Zombie> zombies(zomb);
    	std::vector<Hole> holes(hol);
    	
    	//randomly sets the postion of the man
    	a_man.set_position();
    	
    	//randomly sets the postion of the zombies
    	for(int i = 0; i < zomb; i++)
    	{
    		zombies[i].set_position();
    	}
    
    	//randomly sets the postion of the holes
    	for(int i = 0; i < hol; i++)
    	{
    		holes[i].set_position();
    	}
    	
    	//integer variables for cheking winning and losing
    	int lose = 0;
    	int win = 0;
    
    	//main game loop to move, show and check the objects
    	while(( lose == 0)&&(win == 0))
    	{
    		//displays and check the collision of holes
    	for( int i = 0; i < hol; i++)
    	{
    		holes[i].show_position();
    		if(holes[i].collides_with(a_man))
    		{
    			lose = 1;
    		}
    			//checks the collision of zombies with holes
    		for( unsigned int j = 0; j < zombies.size(); j++)
    		{
    				//if zombie fall into hole it gets removed from the game
    			if(holes[i].collides_with(zombies[j]))
    			{
    				zombies.erase(zombies.begin()+j);
    			}
    		}
    	}
    	//checks the collision of zombies with man
    	for( unsigned int i = 0; i < zombies.size(); i++)
    	{
    		if(zombies[i].collides_with(a_man))
    		{
    			lose = 1;
    		}
    	}
    
    	//shows the postion and moves the zombies
    	for( unsigned int i = 0; i < zombies.size(); i++) 
    	{
    		zombies[i].show_position();
    		zombies[i].movement_Zombie_Hunt(a_man);
    	}
    
    	//if no zombies left in the game
    	if(zombies.size() == 0)
    	{
    		win = 1;
    	}
    
    	//shows position and moves the man
    	a_man.show_position();
    	a_man.movement_Man();
    
    	//clears and refreshes the screen
    	std::cout.sync_with_stdio();
    	Sleep(200);
    	system("CLS");
    	}
    	
    	//checks for a win or loss
    	if(win == 1)
    	{
    		std::cout<< "YOU won" << std::endl;
    	}
    	else
    	{
    		std::cout<< "LOL YOU LOST" << std::endl;
    	}
    }

  2. #2
    Join Date
    Jun 2009
    Location
    France
    Posts
    2,513

    Re: I want to optimize my code

    Your main looks fine, although I would say your class interfaces are discussable (set_position with no argument setting a random position is not a great, imo), but they are minor details.

    Once a program works, if it works it works, there is not much else to say. What sets a "good and optimized" program from the "bad one" is how easy it is to maintain and expand on it.

    I said your main looks fine, but I'd be more interested in your classes: Do Zomby, Man and Hole all derive from an "Placeable object" class? Does Man and Zomby derive from a "Movable Object" class, itself deriving from Placeable? Does moveable_object have a method called "collides_with"? Or did you write it for both zomby and man. Did you write an "enemy_class" from which zomby derives? For when you have new types of enemies, they can derive from enemy_class too?

    Also, you might want to seperate the interface from your objects. Zomby should NOT have a function called show position. Rather, you should have (this is just an example) a class called interface_manager or something, that has a method called update position, and takes a zomby as an argument. Or maybe it would have a method called "refresh all positions".

    These are all ideas. The real question is this:

    If I say to you "Why not add badgers in your game" are you going to answer:

    - "No problem, that's just 5 lines of codes to create an empty badger class, since it derives from "moveable_enemies" where everything is well defined. I can place them inside my vector<moveable_enemy*>, and then that's it. In five minutes, you are playing with badgers."

    - "Aww man! This is going to take forever!"

    There. If you meant optimization as in long loops, or bugs, I noticed that you run all the way to the end of the loop, even if you win or lose. You should break when you find a winning or losing state. No need to run until the end of the program.

    PS: So, how long will it take you to add badgers? What if I tell you they have to move twice as fast as zombies?

  3. #3
    Join Date
    Nov 2009
    Location
    UK
    Posts
    166

    Re: I want to optimize my code

    Quote Originally Posted by monarch_dodra View Post
    Your main looks fine, although I would say your class interfaces are discussable (set_position with no argument setting a random position is not a great, imo), but they are minor details.

    Once a program works, if it works it works, there is not much else to say. What sets a "good and optimized" program from the "bad one" is how easy it is to maintain and expand on it.

    I said your main looks fine, but I'd be more interested in your classes: Do Zomby, Man and Hole all derive from an "Placeable object" class? Does Man and Zomby derive from a "Movable Object" class, itself deriving from Placeable? Does moveable_object have a method called "collides_with"? Or did you write it for both zomby and man. Did you write an "enemy_class" from which zomby derives? For when you have new types of enemies, they can derive from enemy_class too?

    Also, you might want to seperate the interface from your objects. Zomby should NOT have a function called show position. Rather, you should have (this is just an example) a class called interface_manager or something, that has a method called update position, and takes a zomby as an argument. Or maybe it would have a method called "refresh all positions".

    These are all ideas. The real question is this:

    If I say to you "Why not add badgers in your game" are you going to answer:

    - "No problem, that's just 5 lines of codes to create an empty badger class, since it derives from "moveable_enemies" where everything is well defined. I can place them inside my vector<moveable_enemy*>, and then that's it. In five minutes, you are playing with badgers."

    - "Aww man! This is going to take forever!"

    There. If you meant optimization as in long loops, or bugs, I noticed that you run all the way to the end of the loop, even if you win or lose. You should break when you find a winning or losing state. No need to run until the end of the program.

    PS: So, how long will it take you to add badgers? What if I tell you they have to move twice as fast as zombies?

    WOW i do see where you are coming from.
    and i really like your ideas:

    update_position could be a static function? i would just call it and pass my man, hole and zombie as a parameter.

    and set position yeah i should allow to pass a parameter so i could change the range

    and to add badgers it would not take long at all xD

    Body std::vector<Badgers> badgers

    Code:
    #pragma once
    #include"RandomNumber.h"
    
    class Body
    {
    private:
    
    	RandomNumber Random;
    protected:
    	
    	int x;
    	int y;
    	int move;
    	int moved;
    
    public:
    
    	Body(void);
    	~Body(void);
    
    	void movement_Man(void);
    	void movement_Zombie(void);
    	void movement_Zombie_Hunt(Body& );
    
    	bool collides_with( Body& );
    
    	void set_position(void);
    
    	int get_rand(int);
    };
    man.h
    Code:
    #pragma once
    #include"Body.h"
    
    class Man: public Body
    {
    public:
    	void show_position(void);
    };
    it the same for hole and zombie
    Last edited by Mariusmssj; April 16th, 2010 at 05:32 AM.

  4. #4
    Join Date
    Apr 1999
    Posts
    27,449

    Re: I want to optimize my code

    Quote Originally Posted by Mariusmssj View Post
    WOW i do see where you are coming from.
    and i really like your ideas:
    Learn about virtual functions:
    Code:
    void movement_Man(void);
    void movement_Zombie(void);
    void movement_Zombie_Hunt(Body& );
    The use of virtual functions eliminates the need to write code like this. All you would need is this in the base class:
    Code:
    virtual void movement(void);
    And in the derived classes, you override movement() to do whatever that class requires to move. Also, the base class destructor should be virtual once you do that.

    Regards,

    Paul McKenzie

  5. #5
    Join Date
    Jun 2009
    Location
    France
    Posts
    2,513

    Post Re: I want to optimize my code

    I was thinking more something like this:

    Code:
    class Body
    {
    public:
        virtual void move(void)
        {
            //default implementation
        }
    
        virtual bool isHumanInRange(&human)
        {
            //default implementation
        }
    
        virtual void hunt(&human)
        {
            //default implementation
        }
    };
    Code:
    class Man: public Body
    {
    public:
        virtual void move(void)
        {
            //man specific movement
        }
        etc... for other functions
    };
    
    class Zomby: public Body
    {
    public:
        virtual void movement(void)
        {
            //Zomby specific movement
        }
        etc... for other functions
    };
    
    class Badger: public Body
    {
    public:
        void ~movement(void)
        {
            //Badger specific movement
        }
        etc... for other functions
    };
    
    class HunterZomby etc.
    And in your main:

    Code:
    Use containers of specific enemies to mange object lifetime you should not be touching them.
    std::vector<zombies> aZombies;
    std::vector<badgers> aBadgers;
    std::vector<hunterZombies> aHunters;
    etc.
    
    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(player))
        {
            it->hunt(player);
        }
        else
        {
            it->move();
        }
    }
    The basic idea being that you shouldn't care in your main if you are handling Zombies or whatever. Just that they are an enemy. And enemies hunt.

    Of course, they might need more functions, like "hunt" or "is_human_in_range". The badgers could smell the human from further away, and hunt, whereas the zombies would just be moving around aimlessly. The hunter zombies would move faster, or rush when they are hunting.

    The important part is not having to care when you write your main. It should work regardless of if the enemy is a zomby, or a rock/hole (for which movement would simply do nothing...).

    Also, you might want to create an "enemy" in between zomby and body, because it would make no sense for a man to "isHumanInRange".

    Well, these are my opinions anyways. And they are usually easier said than done. If your program works as intended, that's already pretty good.

  6. #6
    Join Date
    Nov 2009
    Location
    UK
    Posts
    166

    Re: I want to optimize my code

    Quote Originally Posted by Paul McKenzie View Post
    Learn about virtual functions:
    Code:
    void movement_Man(void);
    void movement_Zombie(void);
    void movement_Zombie_Hunt(Body& );
    The use of virtual functions eliminates the need to write code like this. All you would need is this in the base class:
    Code:
    virtual void movement(void);
    And in the derived classes, you override movement() to do whatever that class requires to move. Also, the base class destructor should be virtual once you do that.

    Regards,

    Paul McKenzie
    wow this does look nice, but now i am wondering should i even have the man.h, hole.h and zombie.h since all they do is store 1 function to show their position, while i could do it just from the main Body class

  7. #7
    Join Date
    Jun 2009
    Location
    France
    Posts
    2,513

    Re: I want to optimize my code

    Quote Originally Posted by Mariusmssj View Post
    wow this does look nice, but now i am wondering should i even have the man.h, hole.h and zombie.h since all they do is store 1 function to show their position, while i could do it just from the main Body class
    Forget the "1 class 1 file" rule, you can put them all in enemies.h Also, is "show_position really the only thing they will have"?

    What about movement? I think your game isn't an RPG, but what about attack, maybe? Or getHit?

    Remember, there is nothing wrong with little classes. Sometimes, you should even be creating EMPTY classes. Any time you are thinking "this function/class/logic is really small, I might as well just write it in main" you are probably wrong.

  8. #8
    Join Date
    Nov 2009
    Location
    UK
    Posts
    166

    Re: I want to optimize my code

    monarch_dodra just wow, you gave me loads of new ideas xD

    i ran into 2 problems:

    that's in man.cpp

    Last edited by Mariusmssj; April 16th, 2010 at 06:28 AM.

  9. #9
    Join Date
    Jun 2009
    Location
    France
    Posts
    2,513

    Re: I want to optimize my code

    Quote Originally Posted by Mariusmssj View Post
    monarch_dodra just wow, you gave me loads of new ideas xD

    i ran into 2 problems:


    It would appear you have a conflict between your function called "move", and a variable you want to call "move" (which you never declared either).

    PS: Glad I could help

  10. #10
    Join Date
    Nov 2009
    Location
    UK
    Posts
    166

    Re: I want to optimize my code

    ok this is what i got so far:

    and i will add the other characters i just want to know am i on the right track ^^

    Body.h
    Code:
    #pragma once
    #include"RandomNumber.h"
    
    class Body
    {
    private:
    
    	RandomNumber Random;
    protected:
    	
    	int x;
    	int y;
    	int move;
    	int moved;
    
    public:
    
    	Body(void);
    	~Body(void);
    
    	int get_rand(int);
    
            void set_position(int);
    
    	virtual void Move(void);
        virtual bool IsHumanInRange(Body &);
        virtual void Hunt(Body &);
    
    };
    Body.cpp
    Code:
    #include<iostream>
    #include<time.h>
    #include<stdlib.h>
    #include"Body.h"
    #include"RandomNumber.h"
    
    Body::Body(void)
    {
    	int x = 0;
    	int y = 0;
    	int move = 0;
    	int moved = 0;
    }
    
    int Body::get_rand(int upper)
    {
    	return Random.rand_gen(upper);
    }
    
    void Body::set_position(int range)
    {
    	x = get_rand(range);
    	y = get_rand(range);
    }
    
    Body::~Body(void)
    {
    }
    man.h
    Code:
    #pragma once
    #include"Body.h"
    
    class Man: public Body
    {
    public:
    	virtual void move(void);
    	void show_position(void);
    };
    man.cpp
    Code:
    #include<iostream>
    #include"console.h"
    #include"Man.h"
    #include"Body.h"
    
    void Body::Move(void)
    {	
    	std::cin>> move;
    	
    	if((move == 8)&&(moved != 2))
    	{
    		y = y - 1;
    	}
    	else if((move == 2)&&(moved != 8))
    	{
    		y = y + 1;
    	}
    	else if((move == 6)&&(moved != 4))
    	{
    		x = x + 1;
    	}
    	else if((move == 4)&&(moved != 6))
    	{
    		x = x - 1;
    	}
    	moved = move;
    
    }
    void Man::show_position(void)
    {
    	gotoxy(x,y);
    	{
    		std::cout<< "M";
    	}
    }
    zombie.h
    Code:
    #pragma once
    #include"Body.h"
    
    class Zombie: public Body
    {
    public:
    	virtual void Move(void);
    	virtual void Hunt(Body &);
    	void show_position(void);
    };
    Code:
    #include<iostream>
    #include"console.h"
    #include"Zombie.h"
    #include"Body.h"
    
    void Body::Move(void)
    {
    	int choice;
    	choice = get_rand(4);
    
    	if(choice == 0)
    	{
    		y = y - 1;
    	}
    	else if(choice == 1)
    	{
    		y = y + 1;
    	}
    	else if(choice == 2)
    	{
    		x = x + 1;
    	}
    	else
    	{
    		x = x - 1;
    	}
    }
    
    void Body::Hunt(Body & body)
    {
    	if(this->x > body.x)
    	{
    		this->x = this->x - 1;
    	}
    	else if (this->x < body.x)
    	{
    		this->x = this->x + 1;
    	}
    	else if(this->y > body.y)
    	{
    		this->y = this->y - 1;
    	}
    	else if(this->y < body.y)
    	{
    		this->y = this->y + 1;
    	}
    }
    
    void Zombie::show_position(void)
    {
    	gotoxy(x,y);
    	{
    		std::cout<< "Z";
    	}
    }
    a question:
    class Badger: public Body
    {
    public:
    void ~movement(void) why the (~) destructor there?


    and i declined
    virtual void Move(void);
    virtual bool IsHumanInRange(Body &);
    virtual void Hunt(Body &);
    in body but defined them individually in man and zombie. Now i need to define "virtual bool IsHumanInRange(Body &);" in zombie or body?

    thanks, i do get your idea, but since i am still new to C++ i want to ask all the question i can xD sorry if i get annoying
    Last edited by Mariusmssj; April 16th, 2010 at 07:13 AM.

  11. #11
    Join Date
    Apr 1999
    Posts
    27,449

    Re: I want to optimize my code

    Quote Originally Posted by Mariusmssj View Post
    ok this is what i got so far:

    and i will add the other characters i just want to know am i on the right track ^^
    The destructor for the base class should be virtual.

    Code:
    #pragma once
    #include"RandomNumber.h"
    
    class Body
    {
    private:
    
    	RandomNumber Random;
    protected:
    	
    	int x;
    	int y;
    	int move;
    	int moved;
    
    public:
    
    	Body(void);
    	virtual ~Body(void);
    Regards,

    Paul McKenzie

  12. #12
    Join Date
    Nov 2009
    Location
    UK
    Posts
    166

    Re: I want to optimize my code

    Quote Originally Posted by Paul McKenzie View Post
    The destructor for the base class should be virtual.

    Code:
    #pragma once
    #include"RandomNumber.h"
    
    class Body
    {
    private:
    
    	RandomNumber Random;
    protected:
    	
    	int x;
    	int y;
    	int move;
    	int moved;
    
    public:
    
    	Body(void);
    	virtual ~Body(void);
    Regards,

    Paul McKenzie
    so my .cpp will not change?

    Code:
    Body::~Body(void)
    {
    }
    i will delete all the dynamic pointers in main

  13. #13
    Join Date
    Jun 2009
    Location
    France
    Posts
    2,513

    Re: I want to optimize my code

    Quote Originally Posted by Mariusmssj View Post
    void ~movement(void) why the (~) destructor there?
    Copy paste typo, sorry.

    Quote Originally Posted by Mariusmssj View Post
    and i declined
    virtual void Move(void);
    virtual bool IsHumanInRange(Body &);
    virtual void Hunt(Body &);
    in body but defined them individually in man and zombie. Now i need to define "virtual bool IsHumanInRange(Body &);" in zombie or body?

    thanks, i do get your idea, but since i am still new to C++ i want to ask all the question i can xD sorry if i get annoying
    The basic idea is to define a default behavior in the parent class (in this case body), and create a specific behavior in the derived class, if the behavior is different from the default.

    In your example, I would put the "walk around randomly" implementation in "body". I would redefine it for man (since it has user interaction). Zomby and Badger would not redefine it, since the default is fine. HunterZombie would redefine it, since it walks twice as fast (or you can define a speed variable...)

  14. #14
    Join Date
    Nov 2009
    Location
    UK
    Posts
    166

    Re: I want to optimize my code

    Quote Originally Posted by monarch_dodra View Post
    Copy paste typo, sorry.



    The basic idea is to define a default behavior in the parent class (in this case body), and create a specific behavior in the derived class, if the behavior is different from the default.

    In your example, I would put the "walk around randomly" implementation in "body". I would redefine it for man (since it has user interaction). Zomby and Badger would not redefine it, since the default is fine. HunterZombie would redefine it, since it walks twice as fast (or you can define a speed variable...)
    so for zombie "move" should go in "body"

    and by redefine man i am not too sure what you mean, sorry "/

  15. #15
    Join Date
    Jun 2009
    Location
    France
    Posts
    2,513

    Re: I want to optimize my code

    Quote Originally Posted by Mariusmssj View Post
    so for zombie "move" should go in "body"

    and by redefine man i am not too sure what you mean, sorry "/
    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.

Page 1 of 2 12 LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured