design dilemma

[TheFerryman]

Hi,

I keep running up against the same design problem. Let's say I have a
cat object like this:

//-----------------------------------------------------------------------------------------
class Cat
{
private:

Vision* m_pVision;

Memory* m_pMemory

public:

//update vision and add all visible cats to memory
void Update(){m_pVision->Update(this);}

const Memory* GetMemory()const{return &m_pMemory;}
};

//-----------------------------------------------------------------------------------------

The Vision class looks like this:

//-----------------------------------------------------------------------------------------
class Vision
{
//returns a vector of all other cats pCat can see
vector<Cat*> GetAllOtherAnimalsInView(Cat* pCat);

void Update(Cat* pCat)
{
pCat->GetMemory()->Add(GetAllOtherAnimalsInView(pCat));
}
};

//-----------------------------------------------------------------------------------------

This is where the problem occurs, because GetMemory() must be non
const for Vision::Update to use it. And of course, if I make it non
const then there's no reason to have it private in the first place.

An alternative would be for Cat to delegate the call to Memory::Add,
by having its own Add method like so:

void Cat::Add(vector<Cat*> cats){m_pMemory->Add(cats);}

but as classes become more complex the number of extra methods I'd
have to include may get silly and I'd be better off merging the two
classes together (ie. The Memory class merged into the Cat class)

I seem to be getting this design issue fairly regularly and -- shame
on me -- I just end up creating a const and a non const version of the
call to obtain a member instance... which seems to me like very poor
design.

How should I handle this problem?

Many thanks for any feedback.

[David Rubin]

TheFerryman wrote:

class Cat
{
private:
Vision* m_pVision;
Memory* m_pMemory

public:
//update vision and add all visible cats to memory
void Update(){m_pVision->Update(this);}

const Memory* GetMemory()const{return &m_pMemory;}
};
[snip] class Vision
{
//returns a vector of all other cats pCat can see
vector<Cat*> GetAllOtherAnimalsInView(Cat* pCat);

void Update(Cat* pCat)
{
pCat->GetMemory()->Add(GetAllOtherAnimalsInView(pCat));
}
}; This is where the problem occurs, because GetMemory() must be non
const for Vision::Update to use it. And of course, if I make it non
const then there's no reason to have it private in the first place.

I think you have to define your problem in plain English. This is what I
think you want to model:

You have a class Cat. Each Cat has a Vision (a list of other Cats it can
see), and a Memory (a list of Cats that it sees and remembers?) The
method Cat::update() adds the Cat's Vision to Memory.

If this is accurate, I think you want something more like this:

class Cat
{
private:
Vision *m_vision;
Memory *m_memory;
public:
void update() { m_memory->add(m_vision); }
/* ... */
}

Where Vision has a public method which returns a vector<Cat*>, or
whatever you prefer.

If Vision and Memory do nothing more than store lists of Cat*'s, you
could define them as

std::dequeue<Cat*> m_vision;
std::dequeue<Cat*> m_memory;

(or an appropriate typedef) and then

void update() {
m_memory.insert(m_memory.end(),
m_vision.begin(),
m_vision.end());
}

/david

--
"As a scientist, Throckmorton knew that if he were ever to break wind in
the echo chamber, he would never hear the end of it."

[TheFerryman]

On Thu, 04 Mar 2004 21:14:59 GMT, David Rubin <fu******@warpmail.net>
wrote:

TheFerryman wrote:

class Cat
{
private:
Vision* m_pVision;
Memory* m_pMemory

public:
//update vision and add all visible cats to memory
void Update(){m_pVision->Update(this);}

const Memory* GetMemory()const{return &m_pMemory;}
};

[snip]

class Vision
{
//returns a vector of all other cats pCat can see
vector<Cat*> GetAllOtherAnimalsInView(Cat* pCat);

void Update(Cat* pCat)
{
pCat->GetMemory()->Add(GetAllOtherAnimalsInView(pCat));
}
};

This is where the problem occurs, because GetMemory() must be non
const for Vision::Update to use it. And of course, if I make it non
const then there's no reason to have it private in the first place.

I think you have to define your problem in plain English. This is what I
think you want to model:

You have a class Cat. Each Cat has a Vision (a list of other Cats it can
see), and a Memory (a list of Cats that it sees and remembers?) The
method Cat::update() adds the Cat's Vision to Memory.

If this is accurate, I think you want something more like this:

class Cat
{
private:
Vision *m_vision;
Memory *m_memory;
public:
void update() { m_memory->add(m_vision); }
/* ... */
}

Where Vision has a public method which returns a vector<Cat*>, or
whatever you prefer.

If Vision and Memory do nothing more than store lists of Cat*'s, you
could define them as

std::dequeue<Cat*> m_vision;
std::dequeue<Cat*> m_memory;

(or an appropriate typedef) and then

void update() {
m_memory.insert(m_memory.end(),
m_vision.begin(),
m_vision.end());
}

/david

Hi david,

The Cat/Vision/Memory example is contrived to try to help me
illustrate the type of problem I keep coming up against. In reality
the Memory and Vision classes would have much larger interfaces... a
simple container would not be representative of this type of class at
all.

[David Rubin]

TheFerryman wrote:

On Thu, 04 Mar 2004 21:14:59 GMT, David Rubin <fu******@warpmail.net>
wrote:

TheFerryman wrote:

class Cat
{
private:
Vision* m_pVision;
Memory* m_pMemory

public:
//update vision and add all visible cats to memory
void Update(){m_pVision->Update(this);}

const Memory* GetMemory()const{return &m_pMemory;}
};
[snip]

class Vision
{
//returns a vector of all other cats pCat can see
vector<Cat*> GetAllOtherAnimalsInView(Cat* pCat);

void Update(Cat* pCat)
{
pCat->GetMemory()->Add(GetAllOtherAnimalsInView(pCat));
}
};

[snip - try...]class Cat
{
private:
Vision *m_vision;
Memory *m_memory;
public:
void update() { m_memory->add(m_vision); }
/* ... */
}

Where Vision has a public method which returns a vector<Cat*>, or
whatever you prefer.

[snip - Vision/Memory are simple containers] The Cat/Vision/Memory example is contrived to try to help me
illustrate the type of problem I keep coming up against. In reality
the Memory and Vision classes would have much larger interfaces... a
simple container would not be representative of this type of class at
all.

I didn't really think they were. In any case, The interface you
presented seems to take a very roundabout route to get a Cat's Vision
into its Memory. I'd be interested to know whether the simplified
interface suggested above meets your requirements. As I said, I had
trouble understanding exactly what you are trying to model.

/david

--
"As a scientist, Throckmorton knew that if he were ever to break wind in
the echo chamber, he would never hear the end of it."

[Cy Edmunds]

"TheFerryman" <fe***@onthenet.com> wrote in message
news:sm********************************@4ax.com...

Hi,

I keep running up against the same design problem. Let's say I have a
cat object like this:

//--------------------------------------------------------------------------
--------------- class Cat
{
private:

Vision* m_pVision;

Memory* m_pMemory

public:

//update vision and add all visible cats to memory
void Update(){m_pVision->Update(this);}

const Memory* GetMemory()const{return &m_pMemory;}
};
This design doesn't make sense to me. Look at the public interface:

class Cat
{
public:
void Update();
const Memory *GetMemory() const;
};

Neither method takes an argument! How can Update do anything useful without
input? C++ classes are not so intuitive.

Adding an accessor to a (non-const) Memory * would allow you to code
Update(), but the fact that it depends on a vector of Cat pointers would be
concealed from the client. Unless you are implementing some design pattern
which requires such trickery, I would propose the simpler:

void Update(const std::vector<Cat*> &);

or even (since Update isn't virtual) the more flexible

template <typename CAT_ITER>
void Update(CAT_ITER first_cat, CAT_ITER last_cat);

//--------------------------------------------------------------------------
---------------
The Vision class looks like this:

//--------------------------------------------------------------------------
--------------- class Vision
{
//returns a vector of all other cats pCat can see
vector<Cat*> GetAllOtherAnimalsInView(Cat* pCat);

void Update(Cat* pCat)
{
pCat->GetMemory()->Add(GetAllOtherAnimalsInView(pCat));
}
};

//--------------------------------------------------------------------------
---------------
This is where the problem occurs, because GetMemory() must be non
const for Vision::Update to use it. And of course, if I make it non
const then there's no reason to have it private in the first place.
Why do you say that? If you make m_pMemory public anybody could change the
actual value of the pointer. Ouch. If you add

Memory* GetMemory();

they could only change what the pointer points to. That's a much less
drastic step.

An alternative would be for Cat to delegate the call to Memory::Add,
by having its own Add method like so:

void Cat::Add(vector<Cat*> cats){m_pMemory->Add(cats);}

but as classes become more complex the number of extra methods I'd
have to include may get silly and I'd be better off merging the two
classes together (ie. The Memory class merged into the Cat class)
Well, I can't tell from your description what form this added complexity
will take. The usual way to make things extendable in C++ is to defer as
much as possible to virtual functions. But it's not the only way. I
recommend you have a long look at the Design Patterns book.

I seem to be getting this design issue fairly regularly and -- shame
on me -- I just end up creating a const and a non const version of the
call to obtain a member instance... which seems to me like very poor
design.

Actually it's pretty common. What is says is, "If you have full access to
me, you get full access to my buddy over here too. But if you only have read
access to me you only get read access to my buddy." I wouldn't necessarily
call that poor design although in some cases it might not be appropriate of
course.

[snip]

--
Cy
http://home.rochester.rr.com/cyhome/