Proper use of templated containers as class members

[Per]

I am finding myself doing the following rather often when I have
template containers in classed, e.g. stl::map

For example:

class Foo
{
public:
typedef std::map<std::string, int> direcory_t;
//More typedefs of the same style...
private:
directory_t directory_m;
//More members of the same style...
public:
//Constructors and stuff
const directory_t& directory() const {return directory_m;}
};

The question is what is your opinion on typedefing like this. At the
same time as it saves a lot of typing. After a while there are very
many types in a project. And in the end can get confusing about what
the type really contains. Classes should of course hide their
implementation but it seams very stupid not to have read access
directly to a member. If I didn't have that I have to bloat the class
with wrapper functions for the members. And not using typedefs would
lead to an awful lot of typing.

 

[Maxim Yegorushkin]

I use this approach everywhere.  It's an abstraction.  The users of your
class will use "Foo::directory_t" when they need the type.  The main
thing is not saving typing (although it's a nice side effect) but the
ability to change what 'directory_t' means with a single line, and not
to worry about changing the rest of the code that uses it.  That's the
whole idea behind typedefs.  It doesn't matter how they are defined, as
a namespace member or as a class member.

More often such an approach is called abstraction leak.

The interface of directory_t "abstraction" is that of std::map<>. You
can only change the type of directory_t to something that supports the
full interface of std::map<>, otherwise you break the client code (you
may be lucky if there was limited use of std::map<> directory_t
interface, so that recompilation after changing the type of
directory_t succeeds, but you can't count on that).

 

[Maxim Yegorushkin]

I also use this technique quite a bit, though usually with iterators,
rather than container types.


There is the potential for leakage under certain circumstances, but
there is no abstraction leak in the posted code.  For you to list any
such problem with the code, you're going to have to start with "what if...."


The problem you're describing applies when the amount of client code
that can access the exposed type is unbounded.  When the code is already
encapsulated within some intermediate layer, e.g. it is an
implementation detail of some stand-alone application, there is no need
to enforce any tighter enapsulation.  The public typedef for the
otherwise private type is worth having in such a scenario, not because
it limits potential uses of the type, but because it serves to identify
them.

The literal text "std::map<std::string,int> tells the reader what type
is in use, but Foo::directory_t gives the more relevant information of
why this is the right type for the job.  If ever you do need to change
the type's interface, it's a lot easier to search for Foo::directory_t
than to figure out which instances of std::map<std::string,int> are
relevant to Foo.

If you break the interface in a way that the compiler catches, within
the context of implementing an application, it's not a particularly big
deal.  The potential to break the interface in ways the compiler cannot
catch, e.g. changing the big-O complexity of an operation, is no less
prevalent if a custom type is used rather than a typedef.

My point was that it was not an abstraction, because it abstracts away
nothing but the name of the type. Rather a convenience typedef to make
code less fragile.

Sorry for nor being clear on this.

 

[Per]

Think of the class Foo as an exporter of some service.

Is that service just "work as a holder of a member of
type directory_t"? Sometimes that is appropriate.
It is certainly not a good idea to prohibit such things
as sometimes such a simple "bag of data" is very useful.

But maybe what should happen is that you refactor a
bit and so keep all knowledge of the directory_t
type object inside the class Foo. Maybe the code
that needs to know that such a type, and a member
of that type, exists, maybe all that code should be
inside the class Foo. (Or possibly a very tightly
related small collection of classes. And then you
probably want to look up the proper uses of friend.)

Hmm yes thats a thought, but I don't always know what the users of the
class is beforehand since its currently an evolving research project.
Often an stl container or two is a good structure for what service the
class provides, plus some extra features. So hiding types would lead
to an awful lot of extra work reimplementing the interface of the
container. And using the friend approach im not comfortable with
either, this would mean some classes needs to know about other classes
and thus not getting a clear separation between the classes.

Of course I try to hide as much of the representation as possible. Im
not completely new to OO-design, perhaps this is why this approach has
been nagging me and hence the question here.

That way, the class Foo could become an exporter of
a higher level service such as (from the name
"directory") "look after all the tasks required
to deal with a telephone directory". (Or whatever
it's a directory of.) Then the typedef is not used
outside the class.
Socks

Well if it was as easy as a telephone directory things would be
pretty clear-cut. But my project is a bit more complicated than that


Thanks for all your thoughts so far.

 

[Puppet_Sock]

I am finding myself doing the following rather often when I have
template containers in classed, e.g. stl::map

For example:

class Foo
{
public:
   typedef std::map<std::string, int> direcory_t;
   //More typedefs of the same style...
private:
   directory_t directory_m;
   //More members of the same style...
public:
   //Constructors and stuff
   const directory_t& directory() const {return directory_m;}

};

The question is what is your opinion on typedefing like this. At the
same time as it saves a lot of typing. After a while there are very
many types in a project. And in the end can get confusing about what
the type really contains. Classes should of course hide their
implementation but it seams very stupid not to have read access
directly to a member. If I didn't have that I have to bloat the class
with wrapper functions for the members. And not using typedefs would
lead to an awful lot of typing.

Victor's advice is excellent, as usual.

Maxim and Jeff discussing this produced a thought.
Do you in fact need to expose this through the
interface of the class? Essentially what you have
there is a "getter" function.

const directory_t& directory() const {return directory_m;}

This exposes that the class contains something of
type directory_t, and so needs to expose the type also.
In principle, it exposes that there is a data member
of that type. (Though, in general, it only implies that
the class can proffer up data in that form. Recall the
evergreen discussions about a point class that can
provide x-y data or r-theta data.)

Usually what this means is that you have a fairly
thin abstraction in the class. That is, you've let
some of the functionality to do with the directory_t
type members get outside the class.

Think of the class Foo as an exporter of some service.

Is that service just "work as a holder of a member of
type directory_t"? Sometimes that is appropriate.
It is certainly not a good idea to prohibit such things
as sometimes such a simple "bag of data" is very useful.

But maybe what should happen is that you refactor a
bit and so keep all knowledge of the directory_t
type object inside the class Foo. Maybe the code
that needs to know that such a type, and a member
of that type, exists, maybe all that code should be
inside the class Foo. (Or possibly a very tightly
related small collection of classes. And then you
probably want to look up the proper uses of friend.)

That way, the class Foo could become an exporter of
a higher level service such as (from the name
"directory") "look after all the tasks required
to deal with a telephone directory". (Or whatever
it's a directory of.) Then the typedef is not used
outside the class.
Socks

 

[ma740988]

public:
   //Constructors and stuff
   const directory_t& directory() const {return directory_m;}

};

.. Classes should of course hide their

implementation but it seams very stupid not to have read access
directly to a member. If I didn't have that I have to bloat the class
with wrapper functions for the members.

I often wonder about this myself. Frankly this is one case where I
cant see the value added.

 

[Kai-Uwe Bux]

. Classes should of course hide their

I often wonder about this myself. Frankly this is one case where I
cant see the value added.

It is somewhat analogous to using unnamed numerical constants (magic
numbers) vs. named numerical constants. So, as

struct X {

static unsigned long const number_of_cycles = 13;

};

is better than using 13 throughout the code, using X::directory_t instead of
std::map<std::string, int> is better for the same reasons. Think of using
std::map<std::string, int> as "magic typing".

Now, the documentation may even describe directory_t as implementation
defined and conforming to a certain interface of which map<string,int> is a
model. In that case, using map<string,int> as an implementation of
directory_t is not wrong; and client code that relies on this particular
implementation is broken (in the same way as vector<T>::iterator could be
implemented as T* and client code relying on this implementation is
broken). However, it is true that from a quality of implementation point of
view it would probably be better to do something like:

struct directory_t : private map<string,int> {
// some using ... to forward the parts of the interface
// required by directory_t
//
// some constructors
};


Best

Kai-Uwe Bux

 

[James Kanze]

...

I think you've cut an essential element:
directory_t const& directory() const ;
The typedef is public, and is used in the public interface.
Typedef or not, you've actually exposed the fact that your
implementation uses std::map.

};
The question is what is your opinion on typedefing like this.
I use this approach everywhere. It's an abstraction.
More often such an approach is called abstraction leak.
The interface of directory_t "abstraction" is that of
std::map<>. You can only change the type of directory_t to
something that supports the full interface of std::map<>,
otherwise you break the client code
The public typedef for the otherwise private type is worth
having [...], not because it limits potential uses of the
type, but because it serves to identify them.

My point was that it was not an abstraction, because it
abstracts away nothing but the name of the type. Rather a
convenience typedef to make code less fragile.

"Nothing but?" Abstracting the name of the type is a big
deal. When I write Java code, typedef is the single feature I
miss most, for exactly this reason. When I write
std::vector<int>::iterator, it's not just convenient; it's
because I really don't know what the underlying type is.
That's abstraction at its best.

I think Max's point was related to the fact that the typedef was
being used to expose the fact that you use an std::map in the
implementation.

It's not always obvious what the correct solution should be. If
you use the typedef, then you're pretty much committed to not
changing this aspect of the implementation, since client code
will end up depending on it. In more than a few cases, I've
ended up "wrapping" std::vector<>::const_iterator with a class
of my own, because I didn't want to expose the fact that I was
using std::vector<>, but needed to provide an iterator. Most of
the time, the wrapper class was not a random_access_iterator,
but only a forward_iterator. Just to keep my options open.

Similarly, with std::map, I won't provide access to the map per
se; I'll hoist the necessary functionality into my interface,
using real types. (In the case of std::map, I'll often change
the interface in doing so---provide a const operator[], for
example, if that makes sense, etc.)

 

[James Kanze]

No, he didn't. It's a conversion function. Whatever his
implementation uses, the user *can* have it in the form of a
'directory_t' object. There is nothing "exposed" here. It's
the same as having 'asString' member. My implementation
doesn't use a string to represent the value internally, but
you can have the string if you so desire.

He's returning a reference, not a value, so he needs a
directory_t object somewhere.

};
The question is what is your opinion on typedefing like this.
I use this approach everywhere. It's an abstraction.
More often such an approach is called abstraction leak.
The interface of directory_t "abstraction" is that of
std::map<>. You can only change the type of directory_t to
something that supports the full interface of std::map<>,
otherwise you break the client code
The public typedef for the otherwise private type is worth
having [...], not because it limits potential uses of the
type, but because it serves to identify them.
My point was that it was not an abstraction, because it
abstracts away nothing but the name of the type. Rather a
convenience typedef to make code less fragile.
"Nothing but?" Abstracting the name of the type is a big
deal. When I write Java code, typedef is the single feature I
miss most, for exactly this reason. When I write
std::vector<int>::iterator, it's not just convenient; it's
because I really don't know what the underlying type is.
That's abstraction at its best.

I think Max's point was related to the fact that the typedef
was being used to expose the fact that you use an std::map
in the implementation.

If that's so, it's a very weak point. The typedef is there
for the user's convenience. Nothing more, nothing less.

I agree, sort of. The implementation is exposed by the function
returning the reference. Not using a typedef, and returning the
fully named type wouldn't change anything.

It's not always obvious what the correct solution should be.
If you use the typedef, then you're pretty much committed to
not changing this aspect of the implementation, since client
code will end up depending on it. In more than a few cases,
I've ended up "wrapping" std::vector<>::const_iterator with
a class of my own, because I didn't want to expose the fact
that I was using std::vector<>, but needed to provide an
iterator. Most of the time, the wrapper class was not a
random_access_iterator, but only a forward_iterator. Just
to keep my options open.
Similarly, with std::map, I won't provide access to the map per
se; I'll hoist the necessary functionality into my interface,
using real types. (In the case of std::map, I'll often change
the interface in doing so---provide a const operator[], for
example, if that makes sense, etc.)

That's unnecessary IMO and IME. The user does not have direct
access to the contained object (well, *if* the user knows that
I'm returning a reference to the contained object, they can do
'const_cast' and fiddle with the 'map' directly, but that's
*not in the contract* expressed in the class interface).

If you return a reference, it can only be to a "contained
object". Otherwise, how do you manage its lifetime.

 

[James Kanze]

As concrete examples, there are standard library instance
methods that return char const*, but do not necessarily store
instance data in contiguous, null-terminated arrays.
 std::string::c_str() comes to mind.

Really. Do you know of an implementation of std::string which
doesn't store the data in contiguous, null-terminated arrays?

More generally, his function returned a reference. None of the
coding conventions I've seen have ever required a client side
delete of a reference. And unless otherwise stated, I think it
safe to say that the code doesn't depend on the presence of a
garbage collector.

It is also not hard to imagine a wrapper class around
std::time_t, having a c_str() method that delegates to
std::ctime.  In these cases, it is common practice to use a
static (or user-provided) buffer that is potentially
overwritten on each call to the accessor method.  The contract
explicitly states that the pointer is valid only for the
duration of a single statement, and is not re-entrant.

I think you're clutching at straws.

I also think that it isn't really very relevant. The OP asked
about typedef, not about interface design. I think we can
assume that his example was just that, a quickly thrown together
example, and not necessarily anything that reflected a real
design.

 

[James Kanze]

Sure. The first GCC I used, 2.95, only added a terminating
null when an API call made it necessary. No implementation I
know of stores the array in the simple-minded way you're
implying; e.g., CoW implementations often share one array
among multiple string instances. The only case I can think of
in which a c_str really is just returning a pointer-to-member
is when the Small String Optimization has been applied, but
even that pointer could be invalidated by subsequent appends
to the string.

Which is all really irrelevant to the point at hand. The
function c_str returns (or returned) a pointer to the internal
representation of the string in all of these cases, and the next
version of the standard will require it. (There is some
argument that the current version unintentionally requires it as
well.)

[...]

The debate was whether the typedef implies the implementation.
You argued that it did, partially because the OP's accessor
method returned a reference.

I argued that the posted code would expose the implementation,
because it returned a reference. The typedef has nothing to do
with it, except in the case where it is used in place of a
member class or something similar. Nobody's arguing against
typedef; the argument was against exposing the type at the
interface level.