Am I using a constructor correctly?

Started by
16 comments, last by SonicD007 17 years, 6 months ago
I'm still trying to make my damn chicken text game, but thank god I've learned from it. Anyway, I'm trying to get to start using constructors/destructors and I was wondering if I am using the following correctly:
cEnemy::cEnemy(std::string name, int PL, int atk, int def, int ki, int lvl, int zenni)
{
    SetName(name);
    SetPowerlevel(PL);
    SetMaxPowerlevel(PL);
    SetAttack(atk);
    SetMaxAttack(atk);
    SetDefense(def);
    SetMaxDefense(def);
    SetKi(ki);
    SetMaxKi(ki);
    SetLevel(lvl);
    SetZenni(zenni);
}
I still need to add some code that will create an instance of cEnemy(did I say what I mean to say right?) which I think I will put in the constructor. So, is this the way to go? and is it possible to delete an instance?
Advertisement
using constructors you have direct access to the member variables, so if I were you I would just set their values directly (eg. instead of SetZenni() m_iZenni = zenni). But other than that, you are using your constructor correctly.
You should definately *not* be creating an instance of cEnemy in the cEnemy constructor. A cunstructor is what's called when an object is created. When it's called, there already is an instance being made somewhere, so you don't have to make one; just fill it in.

Also the fact that you have a Set function for so many things seems suspect to me. Do you really need user's to be able to change all of these things?
The code you've shown is fine, and safe. I would recommend actually assigning values to the variables rather than going through a function. It's faster that way, not significantly, but you're saving a function call. This is important when you're looping and such... You could also inline the functions to speed that up...

You can also use "initialization lists," or whatever they're called. Basically, instead of making the data and then assigning it a value, it initializes the data with a value. Code:

MyClass::MyClass(std::string theClassName,int theClassLifeTime) : name(theClassName), lifeTime(theClassLifeTime){  //Do other stuff here...}
We should do this the Microsoft way: "WAHOOOO!!! IT COMPILES! SHIP IT!"
Too many getters and setters looks like a flawed design to me. A class should encapsule behaviour, not behave just like a data bucket except that there's a get and set function instead of a public variabele. Only provide necessary functionality, don't allow anyone to get and set just any var that's inside, if you want to make the most of classes.

As for that constructor, as the above posters already pointed out, there's no need to call set functions as you've got direct access already. But, there's an even better way than assigning these values, and that's using an initialization list.

First an example of assigning:

Woogle::Woogle(string n, int l, int s){name = n;life = l;size = s;}


and now an example of an initialization list:

Woogle::Woogle(string n, int l, int s): name(n), life(l), size(s){}


Woah, what's going on there? Well, what actually happens during a constructor is that all member variabeles constructors are called first. So, the string name would be constructed, and it's default value is simply "". Same goes for life and size: they're initialized at their default value. Then, in the constructors body, you assign values to them: name gets the value of n, and so on.
Using an initialization list allows you to call those constructors directly, so you can skip the whole assigning part because you simply initialize those variabeles at the desired values already.

EDIT: Beaten to it. Well, at least there's some explanation... :)
Create-ivity - a game development blog Mouseover for more information.
Ohhhh. Thats cool. I never even heard of that until now. Also, the get and set functions were for the program to use to give a character power levels and all that. If that's bad coding please point me in a direction on how to improve it because I don't want to get any bad habbits.
In this case there may in fact be a need to have a getter and setter for every member. All we are saying is that don't go giving every member a getter and setter all the time, just think about whether they need it or not.
one useful thing to do in a constructor (in C/C++) is dynamic memory allocation.

for example:
class IntArray {private:int m_size;int *m_data;public:    IntArray(int size) : m_size(size) {        m_data = new int[m_size];    }    ~IntArray() { //destructor        delete m_data; //free up the used memory    }}


sorry for not separating the implementation, i was lazy.
[size="1"]I don't suffer from insanity, I'm enjoying every minute of it.
The voices in my head may not be real, but they have some good ideas!
I'm trying to do that initialization thingy, but I don't understand why it doesn't get the Player variables. Heres what I have and the error:
cEnemy::cEnemy(std::string name, int PL, int atk, int def, int ki, int lvl, int zenni) : P_Name(name), P_Powerlevel(PL), P_Attack(atk), P_Defense(def), P_Level(lvl), P_Zenni(zenni){    }


Compiling: Enemy.cppEnemy.cpp: In constructor `cEnemy::cEnemy(std::string, int, int, int, int, int, int)':Enemy.cpp:15: error: class `cEnemy' does not have any field named `P_Name'Enemy.cpp:15: error: class `cEnemy' does not have any field named `P_Powerlevel'Enemy.cpp:15: error: class `cEnemy' does not have any field named `P_Attack'Enemy.cpp:15: error: class `cEnemy' does not have any field named `P_Defense'Enemy.cpp:15: error: class `cEnemy' does not have any field named `P_Level'Enemy.cpp:15: error: class `cEnemy' does not have any field named `P_Zenni'Process terminated with status 1 (0 minutes, 1 seconds)6 errors, 0 warnings


Are your P_* variables defined in the cEnemy class or some other class?

This topic is closed to new replies.

Advertisement