References

[Andrea Crotti]

Supposing I have something like this below:
class Packer
{
public:
Packer(int& x) : x(x) {}
int& x;
void packData(char *);
};

class Packet
{
public:
int x;
Packer p;
Packet(int x) : x(x), p(x) {}
};

int main() {
Packet p1(10);
cout << p1.x << " and " << p1.p.x << endl;
p1.x = 11;
cout << p1.x << " and " << p1.p.x << endl;
return 0;
}

so the Packer actually only have a subset of the attributes in Packet
and some more methods.

I thought that this actually would work, but actually I get
10 and 10
11 and 0

but why, should it not "follow" the what it points?
Or at worst keep the old value? Where does the 0 comes from now??

In general I'm not wasting any memory with this right?

 

[Ian Collins]

Supposing I have something like this below:
class Packer
{
public:
Packer(int& x) : x(x) {}
int& x;
void packData(char *);
};

class Packet
{
public:
int x;
Packer p;
Packet(int x) : x(x), p(x) {}
};

int main() {
Packet p1(10);
cout<< p1.x<< " and "<< p1.p.x<< endl;
p1.x = 11;
cout<< p1.x<< " and "<< p1.p.x<< endl;
return 0;
}

so the Packer actually only have a subset of the attributes in Packet
and some more methods.

I thought that this actually would work, but actually I get
10 and 10
11 and 0

but why, should it not "follow" the what it points?
Or at worst keep the old value? Where does the 0 comes from now??

Packer is initialises with the constant 10, not Packet.x.

 

[Andrea Crotti]

Packer is initialises with the constant 10, not Packet.x.

A moment I don't get it now is
class Packer
{
public:
Packer(int& _x) : x(_x) {}
int& x;
void packData(char *);
};

class Packet
{
public:
int x;
Packer p;
Packet(int _x) : x(_x), p(_x) {}
};

int main() {
Packet p1(10);
cout << p1.p.x << endl;
p1.x = 3;
cout << p1.p.x << endl;
return 0;
}

but should not here in the constructor
Packet(int _x) : x(_x), p(_x) {}

call the constructor of Packer to set the value inside?
Do I need a "void setX(const int& _x)" in Packer then otherwise?

 

[Victor Bazarov]

A moment I don't get it now is
class Packer
{
public:
Packer(int& _x) : x(_x) {}
int& x;
void packData(char *);
};

class Packet
{
public:
int x;
Packer p;
Packet(int _x) : x(_x), p(_x) {}
};

int main() {
Packet p1(10);
cout<< p1.p.x<< endl;
p1.x = 3;
cout<< p1.p.x<< endl;
return 0;
}

but should not here in the constructor
Packet(int _x) : x(_x), p(_x) {}

call the constructor of Packer to set the value inside?

For a very short time (while the Packet constructor is running) the
reference inside the Packet: member actually refers to a valid object
(the argument to the constructor of the Packet object). As soon as the
constructor returns, the temporary (the argument) is no more, and the
reference inside the 'p' member becomes invalid. Your program has
undefined behavior if it tries to refer to 'p1.p.x' after 'p1' has been
constructed.

Do I need a "void setX(const int& _x)" in Packer then otherwise?

No, you need to understand what a reference is.

V

 

[Andrea Crotti]

It calls the constructor of Packer. But look at the argument that it
passes: _x is the argument to Packet's constructor. It has the value
10 when Packet's constructor runs, and Packer's constructor stores a
reference to that argument. Once Packet's constructor returns, that
argument doesn't exist any more, and the reference points to
nowhere. As Leigh said, the code should use p(x), to initialize the
Packer object so that it's reference points to the value stored in
Packet and not the the argument passed to the constructor.

By the way, this is very difficult to talk about when everything is named x.

Ok very good now is more clear...
Which brings me to the next question.
Suppose I have the situation above, and the Packer also keeps an
alternative representation of the object.

So it would be nice to avoid to recreate this representation every time
I need, but using a reference how do I know when something is changed??

So what would be a clean solution?
One possibility I guess is to have a "dirty bit" flag which I set to
true every time I modify something and false when I regenerate the
representation.

Is there an even smarter way?
Maybe hashing the whole object when I create it and check the hash again
later?

 

[Andrea Crotti]

There are basically two different approaches. Decide who's in charge:
either the Packer notifies the Packet that the value has changed, or
the Packet can ask the Packer whether the value has changed.

Continuing the silly example

class Packer
{
public:
Packer(int& _x) : x(_x), dirty(true) {}
int& x;
bool dirty;
void packData(char *) {
if (dirty)
// do something
dirty = false;
}
};

class Packet
{
public:
int x;
Packer p;
void setX(int _x) {
x = _x;
p.dirty = true;
}
Packet(int _x) : x(_x), p(x) {}
};

int main() {
Packet p1(10);
cout << p1.p.x << endl;
p1.setX(3);
cout << p1.p.x << endl;
return 0;
}

this I get might work, the only annoying part is to have to set the
dirty bit every time I change something manually.

My questions was if there is something even more immediate.

And actually maybe it's better the other way around, having MORE classes
using the same referneces maybe it's best to ASK to Packet if something
is changed.

 

[Andrea Crotti]

Get rid of shared data between classes and use proper encapsulation -
this will save many headaches in long turn. All data should be private
unless your class is just a bundle of unrelated stuff like std:air.
Example:

class Packer
{
public:
Packer(int _x) : x(_x), dirty(true) {}
private:
int x;
bool dirty;
public:
void packData(char *) {
if (dirty)
// do something
dirty = false;
}
void NotifyXChanged(int new_x) {
x = new_x;
dirty = true;
}
int GetX() const {return x;}
};

class Packet
{
public:
Packer p;
void setX(int _x) {
p.NotifyXChanged(_x);
}
Packet(int _x) : p(x) {}
int GetX() const {return p.GetX();}
};

int main() {
Packet p1(10);
cout << p1.GetX() << endl;
p1.setX(3);
cout << p1.GetX() << endl;
return 0;
}


hth
Paavo

That's actually a nice idea, but what if there are many different
Packer??

In the real code there is one Node, which has some data, and different
Packet type that can be generated from that data.

And those Packet type can actually share some fields, so having them
inside each Packet type might be not very good.

In this case it would be great to have ruby(or python)-like metaclasses
support, but looking on the internet looks like it's not so easy...

 

[Andrea Crotti]

In your example Packer was a member of Packet. If there are many such
members then you have to update/notify them all in Packet::setX(). If
each Packet has to take some action if x changes, then somebody will have
to tell it that x has changed, right? The simplest way is to just call
some function on each Packet. If they are all similar it might make sense
to derive them all from a common abstract virtual base class and store a
container of base class pointers. In this case the notification function
would loop over the container and call a virtual function on each object.
- which would then make the right thing depending on the actual type of
the object.

Ah that's interesting, but I didn't really get how to do in practice.
I mean, how do I create a container containing objects of different type
and loop over it?

There was no Node in your example. I have no idea what your actual design
is, so I cannot really answer very specifically. In general, you have to
make up your mind which classes are holding the actual data, and which
classes are reflecting or interpreting it in some way. Maybe you will
have something like document-view model? Or then a visitor model? Can't
tell as the names like Packer and x do not say much to me.

Well the problems is that the
Node class is in a certain way the entry point of the whole library.
So everything will start from there, and "logically" the things that I
should then Pack and send away are actually properties of this node.

When classes are sharing fields, this usually hints inheritance and
derived classes. But maybe you thought about objects sharing fields (via
references like in your original example)? This no-no, try to avoid this.
If something like this is needed, the shared data should be put in a
separate class which is responsible for maintaining it, and multiple
objects of other classes may contain references or pointers to a single
object of this class. This brings the reference-counted smartpointers
instantly into mind.

The actual goal is not to minimize the memory footprint of the program,
but to make a correct, robust and maintainable application (in the limits
of the budget etc). I bet having some data duplication and robust means
of synchronizing it may work out better than trying to share the common
data via references (in addition to more complex data dependencies this
may cause problems related to object lifetimes and multithreading, for
example).

I also suspect you are making the whole thing overly complicated, but
maybe your task requires it.

The task could be done easier, but I've been asked to do things as
generically as possible, to allow in future implementation of different
algorithms and so on.
So my design should be modular enough and correct, performance is not a
real problem here.

In C++ one uses either compile or run-time polymorphism instead of
metaclasses (i.e. either virtual functions or templates).

hth
Paavo

Yes sure, but it's not so simple and so powerful... But of course it can
be done.

 

[Andrea Crotti]

#include <iostream>
#include <cmath>
#include <vector>
// use a smartpointer for automatic lifetime management.
// a raw pointer would work, but requires more care
#include <boost/smart_ptr.hpp>

class Packet;

class PackerBase {
public:
virtual void NotifyPacketChanged(Packet& p)=0;
virtual ~PackerBase() {}
};
typedef boost::shared_ptr<PackerBase> PackerPtr;

class Packet {
private:
int x_;
std::vector<PackerPtr> packers_;
private:
void NotifyPackers() {
for (size_t i=0; i<packers_.size(); ++i) {
packers_->NotifyPacketChanged(*this);
}
}
public:
void setX(int x) {
x_ = x;
NotifyPackers();
}
Packet(int x) : x_(x) {}
void AddPacker(PackerPtr p) {
packers_.push_back(p);
p->NotifyPacketChanged(*this);
}
int GetX() const {return x_;}
};

class Packer1: public PackerBase {
private:
int y_;
private:
virtual void NotifyPacketChanged(Packet& p) {
y_ = 2*p.GetX();
std::cout << "Packer1 set to: " << y_ << "\n";
}
public:
Packer1(): y_(0) {}
};

class Packer2: public PackerBase {
private:
double z_;
private:
virtual void NotifyPacketChanged(Packet& p) {
z_ = std::sqrt(double(p.GetX()));
std::cout << "Packer2 set to: " << z_ << "\n";
}
public:
Packer2(): z_(0) {}
};

int main() {
Packet p1(10);
p1.AddPacker(PackerPtr(new Packer1()));
p1.AddPacker(PackerPtr(new Packer2()));
std::cout << p1.GetX() << std::endl;
p1.setX(3);
std::cout << p1.GetX() << std::endl;
return 0;
}


Output:

Packer1 set to: 20
Packer2 set to: 3.16228
10
Packer1 set to: 6
Packer2 set to: 1.73205
3

Great very nice now I got it.
But for now I changed idea, I'll follow the simplest way, which is
- keep everything in the "main" class
- generate temporary objects only when needed

I'll see later if it makes sense to do these kind of optimizations...