container access design headache

[TheFerryman]

How bad is it to include a non const accessor to a container of
objects? For instance

template <class WheelType>
class CarBase
{
typedef std::list<WheelType*> WheelList;

private:

//for the sake of this example, assume there can be any number
//of wheels
WheelList m_Wheels;

//other components

public:

const WheelList& GetWheels()const{return m_Wheels;}
WheelList& GetWheels(){return m_Wheels;}

//other car related methods
};


I think this doesn't look good because I may as well make the
WheelList public... but I hate making member variables public unless
the class is a simple data structure (which this isn't, the real-world
examples are complex)

But if I only have the const accessor how should a client gain access
to the wheels so they can change their properties? (If I create an
iterator class to iterate through the objects how is that any
different to allowing public access)

This thing keeps going around and around my head and I think I'm
starting to obsess on the problem so any help will be greatly
appreciated. What do *you* do in these circumstances?

 

[Leor Zolman]

How bad is it to include a non const accessor to a container of
objects? For instance

template <class WheelType>
class CarBase
{
typedef std::list<WheelType*> WheelList;

private:

//for the sake of this example, assume there can be any number
//of wheels
WheelList m_Wheels;

//other components

public:

const WheelList& GetWheels()const{return m_Wheels;}
WheelList& GetWheels(){return m_Wheels;}

//other car related methods
};


I think this doesn't look good because I may as well make the
WheelList public... but I hate making member variables public unless
the class is a simple data structure (which this isn't, the real-world
examples are complex)

But if I only have the const accessor how should a client gain access
to the wheels so they can change their properties? (If I create an
iterator class to iterate through the objects how is that any
different to allowing public access)

This thing keeps going around and around my head and I think I'm
starting to obsess on the problem so any help will be greatly
appreciated. What do *you* do in these circumstances?

I think you're having difficulty because of the interface you've chosen. In
a typical collection of some kind, the "get" and "set" functionality isn't
going to be abstracted in terms of getting and putting "the entire
collection", but rather individual elements of the collection. If you were
to separate out "getting" from "setting", then the result of getting could
be a value (rather than a ref), and setting could be sanity-checked. The
way you treat the wheel list doesn't take advantage of any of that; so for
all intents and purposes, you may as well have m_Wheels be public if that's
all the functionality you want.

I think you need to decide whether CarBase is going to concern itself with
individual Wheels or not. If so, then "get" and "set" functionality should
be provided for individual Wheels if you really want CarBase to have some
added value. If not, then WheelList should be a user-defined class in its
own right, perhaps implemented in terms of a list (or not), so that after
obtaining a handle to it from CarBase via GetWheels, the client would still
have to go thorough WheelLists's (presumably safe) interface to munge with
the Wheel List. Make any sense?
-leor

 

[Peter van Merkerk]

How bad is it to include a non const accessor to a container of
objects? For instance

template <class WheelType>
class CarBase
{
typedef std::list<WheelType*> WheelList;

private:

//for the sake of this example, assume there can be any number
//of wheels
WheelList m_Wheels;

//other components

public:

const WheelList& GetWheels()const{return m_Wheels;}
WheelList& GetWheels(){return m_Wheels;}

//other car related methods
};


I think this doesn't look good because I may as well make the
WheelList public...
but I hate making member variables public unless
the class is a simple data structure (which this isn't, the real-world
examples are complex)

In general would avoid that approach, it exposes the implementation of
the CarBase class too much. If you would decide to use another
container, or decide not to store pointers, it would break code the uses
the CarBase class.

But if I only have the const accessor how should a client gain access
to the wheels so they can change their properties? (If I create an
iterator class to iterate through the objects how is that any
different to allowing public access)

Iterators do a better job of hidding implementation details and give the
CarBase class more control what clients can do and what not. If the
CarBase class returns a reference to m_Wheels, clients can do what event
they like with WheelList, including insert and removing elements. If
CarBase has a (non-const) iterator clients can only manipulate WheelType
objects. The iterator could also control which members of the WheelType
objects can be accessed.

This thing keeps going around and around my head and I think I'm
starting to obsess on the problem so any help will be greatly
appreciated. What do *you* do in these circumstances?

Think carefully about the interface of the CarBase class, and consider
carefully what the clients should be able to do with it. Don't expose
more than you have to, this makes it easier to change the implementation
later.

 

[bartek]

How bad is it to include a non const accessor to a container of
objects? For instance

(...)

When you want to be able to modify the elements held by the container, then
there's no other way than make them exposed. Why do you think it is bad?

But if I only have the const accessor how should a client gain access
to the wheels so they can change their properties? (If I create an
iterator class to iterate through the objects how is that any
different to allowing public access)

This is how containers work. They *hold* objects, not *hide* them. Unless
of course you want it otherwise in your implementation.

 

[Claudio Puviani]

How bad is it to include a non const accessor to a container of
objects? For instance

template <class WheelType>
class CarBase
{
typedef std::list<WheelType*> WheelList;

private:

//for the sake of this example, assume there can be any number
//of wheels
WheelList m_Wheels;

//other components

public:

const WheelList& GetWheels()const{return m_Wheels;}
WheelList& GetWheels(){return m_Wheels;}

//other car related methods
};


I think this doesn't look good because I may as well make the
WheelList public... but I hate making member variables public unless
the class is a simple data structure (which this isn't, the real-world
examples are complex)

But if I only have the const accessor how should a client gain access
to the wheels so they can change their properties? (If I create an
iterator class to iterate through the objects how is that any
different to allowing public access)

This thing keeps going around and around my head and I think I'm
starting to obsess on the problem so any help will be greatly
appreciated. What do *you* do in these circumstances?

I think it's a question of logical design. By just exposing the underlying
WheelList, you're saying that it's perfectly valid for any outside entity to
remove every wheel, or to add 30000 wheels, or to make every wheel a different
size, or to make some elements null pointers but leave them in the list, or make
some of the pointers point to shared objects, and so on. In other words, it's a
free-for-all.

In a clean design, if the WheelList is the responsibility of CarBase, then the
interface should restrict access to only the operations that CarBase allows. For
example, CarBase might enforce the same WheelType on a given axel, or even the
same WheelType for the entire car (though this might be overly restrictive).
CarBase should probably restrict the number of wheels that can be set (the exact
number being provided by a virtual function that's overridden in derived
classes). CarBase should DEFINITELY manage the lifetimes of the objects pointed
to by your WheelType pointers and not allow outside entities to meddle. As a
general rule, a class should take responsibility for all of its data members and
enforce its internal consistency unless it's a mere "holder" like std:air<>.

Claudio Puviani

 

[Siemel Naran]

template <class WheelType>
class CarBase
{
typedef std::list<WheelType*> WheelList;

private:

//for the sake of this example, assume there can be any number
//of wheels
WheelList m_Wheels;

//other components

public:

const WheelList& GetWheels()const{return m_Wheels;}
WheelList& GetWheels(){return m_Wheels;}

//other car related methods
};

But if I only have the const accessor how should a client gain access
to the wheels so they can change their properties? (If I create an
iterator class to iterate through the objects how is that any
different to allowing public access)

As others point out, returning iterators don't allow you to add or delete
wheels. But there's mode. You can have the iterator's operator* return a
smart reference. This is a user defined class that behaves like a builtin
reference for most practical purposes. It typically has a protected
constructor, and overrides operator= as in
WheelList::reference:perator=(const Wheel&) which can do additional
checking, such as throwing an exception if you try to put a wrong type of
wheel on a certain car.

This thing keeps going around and around my head and I think I'm
starting to obsess on the problem so any help will be greatly
appreciated. What do *you* do in these circumstances?

Remember, it's often a tradeoff between perfect design, performance, and
time + budget.