Home Why is the pointer in this nested class missing?
Reply: 1

Why is the pointer in this nested class missing?

adayoegi
1#
adayoegi Published in 2017-12-07 16:24:09Z

Question in short:

class B has a ptr to class C, which has a class D having a ptr to class B

assign class B to an array in class A by copying, expecting to see to ptr points to new instance in the array not original instance, but failed.

I am already able to do some workaround, but I want to know why my original approach fails.

More detailed explainations are as follows, and the code to reproduce the problem is posted as well

Anyone who is able to explain what is going on is appreciated.

There are 6 classes:

class CastInfo //contains a Character*

class Skill //abstract class, contains CastInfo

class Movvement : public Skill

class Move1 : public Movement

class Character //contains a Movement*, which will be Move1* in practice

class Squad //contains an array of Character

with the following relationships:

  1. Character* in CastInfo should point to the Character who owns the Skill which is the owner of CastInfo

  2. when assigning the Skill to Character, the Character* in CastInfo points to that Character

  3. the Character in Squad's array should be copied, so there will be 2 instances and the Character* in CastInfo should also point to Character in Squad's array not original instance

The expecting result is:

  1. move1 != ch1.move1 != squad.ch[0].move1 (this is already satisfied)

  2. ch1.move1->cast_info.caster == &ch1 != squad.ch[0].move1->caster_info.caster (this is the problem)

There are 2 cases (tried) of the output:

In the Squad's constructor,: if using

characters_[i] = characters[i];

the character is correctly copied, but the skill is at same address

move1: 00000270E6093500
ch1: 000000BC6DCFF378
ch1.move1: 00000270E6093E60
ch1.move1->cast_info.caster: 000000BC6DCFF378
squad.ch[0]: 000000BC6DCFF3E0
squad.ch[0].move1: 00000270E6093E60
squad.ch[0].move1->cast_info.caster: 000000BC6DCFF378

if using

characters_[i] = Character(characters[i]);

the character is correctly copied, but the skill is missing (pointing to some weird location)

move1: 00000230FDCEF080
ch1: 00000058A11DF548
ch1.move1: 00000230FDCEF260
ch1.move1->cast_info.caster: 00000058A11DF548
squad.ch[0]: 00000058A11DF5B0
squad.ch[0].move1: 00000230FDCEF0E0
squad.ch[0].move1->cast_info.caster: 00000058A11DF378

In the first case, I guess it is probably because I did not overload operator=, so only address is copied. I tried to overload it but it caused more problem. (Such as when using Builder.Build() )

In the second case, I expect it first call copy constructor, which triggers SetMove1(), which calls SetCaster(). move1 is cloned as shown, but I cannot understand why caster is not updated correctly. (Though is calls operator= after construnction, the address should remain the same.)

The following code should reproduce the problem:

motion.h

#pragma once
class Character;

struct CastInfo
{
    Character* caster;
    int coeff;
};

class Skill
{
public:
    CastInfo cast_info;
    Skill() {};
    ~Skill() {};
    virtual void DoSomething() = 0;
};

class Movement : public Skill
{
public:
    Movement();
    ~Movement();
    virtual void DoSomething() { ; }
    virtual Movement* Clone() const { return new Movement(*this); }
};

class Move1 : public Movement
{
public:
    Move1() { cast_info.coeff = 123; }
    void DoSomething() { ; }
    virtual Move1* Clone() const { return new Move1(*this); }
};

class Move2 : public Movement
{
public:
    void DoSomething() { ; }
};

motion.cpp:

#include "motion.h"
Movement::Movement() { }
Movement::~Movement() { }

test.h:

#pragma once
#include <string>
#include <vector>
#include "motion.h"
#define SQUAD_SIZE 6

extern Movement* null_movement;
class Character
{
public:
    class Builder;
    Character();
    ~Character();
    Character(const Character& character);
    Character& SetMove1(Movement* skill);
public:
    int id_;
    Movement* move1_ = null_movement;
    Movement* move2_ = null_movement;
    Character(int id) : id_(id) { ; }
    void SetCaster();
};

class Character::Builder : public Character
{
public:
    Builder& SetId(int i) { id_ = i; return *this; }
    Character Build() { return Character(id_); }
};

class Squad
{
public: 
    class Builder;
    Squad() { }
    Squad(const Squad& squad);
    ~Squad() { }
public:
    Character characters_[SQUAD_SIZE];
    Squad(Character* characters);
};

class Squad::Builder :public Squad
{
public:
    Builder& SetCharacter(const Character& character, const int position) { characters_[position] = character; return *this; }
    Squad Build() { return Squad(characters_); }
};

test.cpp

#include <iostream>
#include "test.h"

Movement* null_movement = new Move2();
Character::Character() : id_(0) { }
Character::~Character() {}
Character::Character(const Character& character) {
    id_ = character.id_;
    SetMove1(character.move1_);
}

Character& Character::SetMove1(Movement* move1) {
    if (!move1) return *this;
    move1_ = move1->Clone(); 
    SetCaster();
    return *this;
}

void Character::SetCaster() { 
    if (move1_ != NULL) move1_->cast_info.caster = this;
}

Squad::Squad(const Squad& squad) {
    *this = squad;
}

Squad::Squad(Character* characters) {
    for (int i = 0; i < SQUAD_SIZE; i++) {
        //characters_[i] = characters[i];  //character copied, skill same address
        characters_[i] = Character(characters[i]); //character copied, skill missing
    }
}

main.cpp

#include <iostream>
#include "test.h"
#include "motion.h"
int main() {

    Move1* move1 = new Move1();
    std::cout << "move1: " << move1 << std::endl;

    Character ch1 = Character::Builder().SetId(1).Build();
    Character ch2 = Character::Builder().SetId(2).Build();
    ch1.SetMove1(move1);

    std::cout << "ch1: " << &ch1 << std::endl;
    std::cout << "ch1.move1: " << (ch1.move1_) << std::endl;
    std::cout << "ch1.move1->cast_info.caster: " << (ch1.move1_->cast_info.caster) << std::endl;

    Squad squad = Squad::Builder().SetCharacter(ch1, 0).SetCharacter(ch2, 1).Build();

    std::cout << "squad.ch[0]: " << &(squad.characters_[0]) << std::endl;
    std::cout << "squad.ch[0].move1: " << (squad.characters_[0].move1_) << std::endl;
    std::cout << "squad.ch[0].move1->cast_info.caster: " << (squad.characters_[0].move1_->cast_info.caster) << std::endl;

    system("PAUSE");
    return 0;
}

As previously mentioned, I have a workaround to reach my goal:

By creating another method, which iterates through the array in Squad, and call each Character's SetCaster() method.

void Squad::SetCaster() {
    for (int i = 0; i < SQUAD_SIZE; i++) {
        characters_[i].SetCaster();
    }
}

But I think this is dirty because every time after Builder::Builder(), SetCaster() must be called, which is unintuitive and error-prone.

adayoegi
2#
adayoegi Reply to 2017-12-08 01:14:04Z

I think I found the problem, as illustrated below:

The problem is in

Squad::Squad(Character* characters) {
    for (int i = 0; i < SQUAD_SIZE; i++) {
        //characters_[i] = characters[i];  //character copied, skill same address
        characters_[i] = Character(characters[i]); //character copied, skill missing
    }
}

As mentioned in question, using the commented line is just copying the values, which is incorrect.

What

characters_[i] = Character(characters[i]); //character copied, skill missing

does is as follows:

  1. create a Character, by calling Character's constructer, this object is at address A. SetMove1() is called, SetCaster() is called. The pointer in cast_info is pointing at A correctly.

  2. assign the object to characters_[i], whose address is at address B because the address of characters_ is assigned when Squad is created. As I did not overload Character::operator=, the pointer is still pointing to address A

  3. Constructor done, Squad returned.

This is the reason

std::cout << "squad.ch[0].move1->cast_info.caster: " << (squad.characters_[0].move1_->cast_info.caster) << std::endl;

shows a third address (address A) which is neither &(character[0]) (address B) nor &ch1 (original character's address)

The solution is either to overload operator= or put my "workaround" (Squad::SetCaster()) in constructor right after the for loop.

Please correct me if there is anything wrong, or if there is any better solution.

You need to login account before you can post.

About| Privacy statement| Terms of Service| Advertising| Contact us| Help| Sitemap|
Processed in 0.317988 second(s) , Gzip On .

© 2016 Powered by mzan.com design MATCHINFO