c++ forward declaration + ifndef also needs pointers

117 views Asked by At

Array:

#ifndef ARRAY_H
#define ARRAY_H

#include <bits/stdc++.h>
using namespace std;

namespace Maifee{

class Value;

class Array {
public:
    Array();
    vector<Value> _elements;
};

}

#endif // ARRAY_H

Object :

#ifndef OBJECT_H
#define OBJECT_H


#include <bits/stdc++.h>
#include "value.h"
using namespace std;

namespace Maifee{

class Value;

class Object{
public:
    Object();
    map<string, Value> _members;
};

}


#endif // OBJECT_H

Value :

#ifndef VALUE_H
#define VALUE_H

#include <bits/stdc++.h>
#include "array.h"
#include "object.h"
using namespace std;

namespace Maifee{
    
class Array;
class Object;

class Value {
public:
    Value();
    Value(Object *__object);
    Array *_array;
    Object *_object;
};

}

#endif // VALUE_H

I'm learning C++ at my best. With my teeny tiny knowledge in C++, I am trying to write some code. First reason I'm moving C++, pointers take a lot's of time.

Here I'm writing these code, where forward-declaration is necessary, and due to this even after using forward-declaration and ifndef, I need to use pointer, which I really don't want.

Can anyone really help me with this, how can I remove circular dependency??

Do I need to go back to C?

When using pointer I faced many problems such as, I have just one key-value pair in my map, but in the next line size becomes a very large number, out of nowhere.

Code inside main :

    Object object=Object();
    cout << "pop obj tem len" << object._members.size() << endl; //gives 0 as expected
    object._members = members;
    cout << "pop obj tem len" << object._members.size() << endl; //gives the expected number
    Value val=Value(&object);
    cout << val._object->_members.size() << "size here" << endl; //gives a random number

Constructor for Value with Object parameter :

Value::Value(Object *__object)
{
    Object object;
    object._members.insert(__object->_members.begin(), __object->_members.end());
    _object = &object;
}
1

There are 1 answers

7
Remy Lebeau On

You can't avoid the forward-declarations and pointers in this situation.

class Object has a map<string, Value> member, and class Array has a vector<Value> member. Which means Value must be a fully-defined, complete type by the time Object and Array are being compiled, as map and vector need to know the total size of their element types. If Value were to have non-pointer Array and Object members, then Object and Array would need Value to be a complete type, but Value would need Object and Array to be complete types. Catch-22!

So, you have to use forward declarations and pointers/references for the Value members in order to make this kind of circular referencing work properly, since pointers/references to incomplete types are allowed.


UPDATE: In your Value constructor that takes an Object* parameter, you are setting the _object member to point at a local Object instance that goes out of scope and is destroyed when the constructor exits, thus leaving _object dangling. That is why the subsequent val._object->_members.size() expression in main() produces garbage (you are lucky the code didn't crash outright) - val._object is pointing at invalid memory, so its members is not a valid map object and so reading its size() is undefined behavior. That goes right back to the original comment I posted:

You are likely accessing an invalid pointer.

To solve this, depending on your actual design goal, the Value constructor needs to either:

  • construct a new Object dynamically, which will have to be delete'd later. You will also have to provide a proper copy constructor and copy assignment operator:
Value::Value()
{
    _object = NULL;
    _array = NULL;
}

Value::Value(Object *__object)
{
    _object = new Object;
    _array = NULL;
    if (__object)
        _object._members = __object->_members;
}

Value::Value(const Value &__value)
{
    _object = new Object;
    _array = NULL;
    if (__value._object)
        _object._members = __value._object->_members;
}

Value::~Value()
{
    delete _object;
}

Value& Value::operator=(const Value &__value)
{
    if (&__value != this)
    {
        Value tmp(__value);
        std::swap(_object, tmp._object);
        std::swap(_array, tmp._array);
    }
    return *this;
}
  • simply store the Object* pointer it is given:
Value::Value(Object *__object)
{
    _object = __object;
    _array = NULL;
}