views:

159

answers:

4

Hi, I'm new to C++ and I'm trying to figure out this problem I'm having with my constructor for one of my classes. What happens is... all my variables are initialized properly except two (health and type).

#pragma once
#include <irrlicht.h>
#include <vector>
#include <cassert>

using namespace irr;
using namespace core;
using namespace scene;

enum
{
    PLAYER = 0,
    NPC = 1,
    SOLDIER = 2,
    CHAINGUNNER = 3

};

class Model
{
    public:
        Model(void);
        Model(int id, std::vector<ISceneNode*> modelVec, int modType);
        ~Model(void);

        std::vector<int> path;
        std::vector<ISceneNode*> model;
        int endNode;
        int type;
        int animate;
        int health;
        u32 lastAnimation;

    private:
        int mId;
};

#include "Model.h"

Model::Model(void)
{
    //assert(false);
}

Model::Model(int id, std::vector<ISceneNode*> modelVec, int modType)
{
    path = std::vector<int>();
    model = modelVec;
    endNode = 0;
    type = modType;
    animate = 0;
    health = 100;
    lastAnimation = 0;
    mId = id;
}

Model::~Model(void)
{}

I create a model with Model soldier(id, model, SOLDIER) Everything is set properly except type and health. I've tried many different things, but I cannot figure out my problem. I'm not sure but the default constructor is being called. It doesn't make sense because I make no called to that constructor.

Thanks,

vector<ISceneNode*> model;
model.push_back(soldierBody);
model.push_back(soldierHead);
model.push_back(soldierWeapon);

cout << "Id of char: " << id << endl;

Model soldier(id, model, SOLDIER);
modelMap[id] = soldier;
+1  A: 

From the comments, you say that you are inserting these into a map like so:

modelMap[id] = Model(id, model, SOLDIER);

std::map::operator[] requires that the mapped type be default constructible. When you call operator[] on a map, if there is no mapped value with the given key, the map default constructs a new object, maps it to the given key, and returns a reference to that object.

You can get around this by using std::map::insert():

modelMap.insert(std::make_pair(id, Model(id, model, SOLDIER));
James McNellis
This didn't work. It still has the wrong initialized values.
2Real
Where and how do you tell if the values are initialized right?
kotlinski
The best would probably be to just print it to the console, at the end of constructor... Debuggers sometimes show wrong values, or don't break at the right point, especially if optimizations are enabled.
kotlinski
I want the health to be initialized to 100 and the type of the model to be initialized to the type that's passed into the constructor.Both of their values are -858993460 when I print them out.
2Real
@2Real: That is `0xcccccccc` in hex, which indicates an uninitialized variable on the stack (assuming you are using the Visual C++ debug runtime). You need to step through the execution using a debugger to see where that uninitialized value is coming from.
James McNellis
It's coming from calling the default constructor.
2Real
You will have to update the default constructor so that it initializes the values correctly. Then see if the problems disappear.
kotlinski
How am I supposed to set the type then? I can set the health to 100 in the constructor but I can't set the type because it needs to be passed in. Even if I set the type to 0 in the default constructor, that value never seems to be updated.
2Real
@2Real: If the type needs to be passed in, then _don't declare a default constructor_.
James McNellis
I get "c:\program files (x86)\microsoft visual studio 9.0\vc\include\map(173) : error C2512: 'Model::Model' : no appropriate default constructor available"
2Real
See answer for answer :)
kotlinski
@2Real: Pick up __any__ beginner's C++ book and look up how to write a default constructor so that even built-ins are initialized properly. This is so basic, it will likely have been treated in the first lesson/chapter about C++ classes.
sbi
Obligatory link to [The Definitive C++ Book Guide and List](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). I don't really have any more to suggest.
James McNellis
Why isn't the current object being overwritten in the map? I wouldn't need a default constructor if I just did modelMap[id] = soldier; of modelMap.insert(std::make_pair(id, soldier)); Where soldier is a newly created object?
2Real
@2Real: (1) Put a breakpoint in the default constructor, (2) Run your program attached to the debugger, (3) Look at the stack trace. This is debugging 101.
James McNellis
I have and when I call something like Model wagonModel(id, model, NPC), it calls the default constructor and it seems to ignore any initializations I do in the parametrized constructor.
2Real
What if you remove the default constructor and everything that uses it? Including map[]
kotlinski
Do you guys think it a problem with copying the object over into the map? Do I have to override something? The object is being created with the right values. They just seem to not translate over to the map.
2Real
2Real: `<sigh>` This is really __very__ basic and you really should read it up yourself, but here you go: Your `m[k]=v;` is an _assignment expression_ that consists of two _sub-expressions_, the access into the map `m[k]` left of the `=` and `v` (a rather simple expression) on the right of it. In order to perform the assignment, both sub-expressions already have to be evaluated. (The order is unspecified.) For the right side, that's very simple. For the left side, `m[k]` will search the map for an entry with the key `k` __and return a reference to it__....
sbi
...Since this leaves no room for reporting failures, `m[k]` __has to successfully return a valid reference to an existing value for an entry with the key `k`__. If that entry already exists, it will return a reference to the existing value. If it doesn't exist, `m[k]` will _create an entry with the key `k` and a_ __default-constructed object__, and return the reference to that. That's why types used as map values need a default-constructor when accessed using the map's `operator[]`. Note that this also means that entering value into a map through `operator[]` wastes performance.
sbi
+1  A: 

You do:

Model soldier(id, model, SOLDIER); //1
modelMap[id] = soldier;            //2

What happens here?
1. New object is created, using consructor you have provided.
2. The so-called copy-constructor copy assignment operator is called to copy soldier to modelMap[id]. You haven't defined your own copy-constructor copy assignment operator so one default is created for you by compiler, it is in most cases just copying byte-by-byte whole data-structure to the new memory address. However you have vector of pointers in your class, so compiler should call copy-constructor of vector... And I don't know (maybe someone with greater experience would know exactly what happens now) what is the result copy-constructor, I don't know if standard clearly defines 'default copy-constructor'.

So it is possible, that the whole structure is copied to the modelMap[] but with some random data.

If you create a copy-constructor (its declaration in your case will look something like Model::Model(const Model& myModel);, copy-constructor always takes reference to object of its type as an argument) If you override copy assignment operator (best, if you make both things), you have control over everything that is done while copying your object to another variable/object.

Download eg. Bruce Eckel's Thinking in C++, V. 1 [1], or search somewhere on the Net how to do it (probably this will be good, didn't read whole article, http://www.learncpp.com/cpp-tutorial/911-the-copy-constructor-and-overloading-the-assignment-operator/).

[1] Downloadable on his website, mindview.net, as a new user I can paste only one link, so cannot link it here myself :P.

silmeth
2Real wrote as a comment to the another answer: *Do you guys think it a problem with copying the object over into the map? Do I have to override something? The object is being created with the right values. They just seem to not translate over to the map*So the data is being broken while copying. IMO copy-constructor would fix the issue.
silmeth
(2) does not invoke the copy constructor; it invokes the copy assignment operator. Since the class in question does not have a user-declared copy assignment operator, the default copy assignment operator is provided by the compiler; this operator simply copies each member from the right hand side to the left hand side object.
James McNellis
Right, my bad. However, the mechanism is (almost?) the same.
silmeth
+1  A: 

This lines:

modelMap[id] = soldier;

First default constructs the Model inside the map.
The returned reference is then used with the assignment operator to copy the value of soldier into the value contained inside the map.

To test if it is working try:

Model soldier(id, model, SOLDIER);
std::cout << "TYPE(" << soldier.type << ")  HEALTH(" << soldier.health << ")" std::endl;

modelMap[id] = soldier;
std::cout << "TYPE(" << modelMap[id].type << "  HEALTH(" << modelMap[id].health << ")" std::endl;

If your class is not designed to be default constructible.
Then do not have a default constructor (this will just lead to problems).
Declare a default constructor in the private part of the class (no need for a body).

Without a default constructor you will not be able to use the operator[] on map. But you can get around this by using insert:

modelMap.insert(std::map<XX, Model>::value_type(id, soldier));
Martin York
There is no need to declare a private default constructor. It will only be generated if no other constructor has been declared.
kotlinski
A: 

Rebuild the solution.

Basilevs