Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > inheriting from std::vector bad practice?

Reply
Thread Tools

inheriting from std::vector bad practice?

 
 
James Kanze
Guest
Posts: n/a
 
      04-03-2010
On Apr 3, 7:20 pm, "Alf P. Steinbach" <(E-Mail Removed)> wrote:
> * Leigh Johnston:


[...]
> > 3. A standard library container destructor is not virtual so
> > you cannot delete the associated object via a pointer to the
> > container (base class).


> >http://www.i42.co.uk/stuff/mutable_set.htm


> Well, the most important one is still missing:


> 0. Is the class derivation an implementation detail?


> If so then exposing it as public, even as a public member instead of
> as a base class,


> 0.A. Introduces needless ways that bugs can creep in. E.g., when deriving
> from std::vector<T>, then client code may use iterators incorrectly.


In what way? IIUC, the argument is that the base abstraction
*is* std::vector<Point2D>. In that case, all that the
derivation does is introduce a few extra convenience functions.
And while the idiomatic way of doing this in C++ is by means of
free functions, rather than derivation, I don't see how using
derivation for this can possibly lead to something like misuse
of the iterators.

--
James Kanze
 
Reply With Quote
 
 
 
 
Alf P. Steinbach
Guest
Posts: n/a
 
      04-03-2010
* James Kanze:
> On Apr 3, 5:16 pm, "Alf P. Steinbach" <(E-Mail Removed)> wrote:
>> * Leigh Johnston:
>>> "Alf P. Steinbach" <(E-Mail Removed)> wrote in message
>>> news:hp7p5m$6rb$(E-Mail Removed)-september.org...
>>> I repeat: interface augmentation (which is what my reply was referring
>>> to) is quite valid

>
>> Sometimes interface augmentation is a good idea, but in this
>> case there is no interface augmentation.

>
> Strangely enough, I have to agree with Leigh here. (Doesn't
> happen very often.) From what little we know of the global
> context, this seems very much like what I would call "interface
> augmentation". My only objection to the derivation is that in
> C++ (unlike the case in e.g. Java), the idiomatic form of
> interface augmentation is by using free functions: we have
> std::sort, rather than std::vector<>::sort, etc.


In addition to the code itself we have the OPs statement that std::vector<Point>
is just an arbitrary implementation detail, where all he uses is push_back.

It's not an intended interface.

And with no intended interface there's no augmentation.


Cheers & hth.,

- Alf
 
Reply With Quote
 
 
 
 
Alf P. Steinbach
Guest
Posts: n/a
 
      04-03-2010
* James Kanze:
> On Apr 3, 7:20 pm, "Alf P. Steinbach" <(E-Mail Removed)> wrote:
>> * Leigh Johnston:

>
> [...]
>>> 3. A standard library container destructor is not virtual so
>>> you cannot delete the associated object via a pointer to the
>>> container (base class).

>
>>> http://www.i42.co.uk/stuff/mutable_set.htm

>
>> Well, the most important one is still missing:

>
>> 0. Is the class derivation an implementation detail?

>
>> If so then exposing it as public, even as a public member instead of
>> as a base class,

>
>> 0.A. Introduces needless ways that bugs can creep in. E.g., when deriving
>> from std::vector<T>, then client code may use iterators incorrectly.

>
> In what way? IIUC, the argument is that the base abstraction
> *is* std::vector<Point2D>.


It isn't in the case we were discussing, and it certainly isn't for the context
of point 0.A above where the class derivation is an implementation detail.


Cheers & hth.,

- Alf
 
Reply With Quote
 
James Kanze
Guest
Posts: n/a
 
      04-03-2010
On Apr 3, 10:23 pm, "Leigh Johnston" <(E-Mail Removed)> wrote:
> "James Kanze" <(E-Mail Removed)> wrote in message


[...]
> I don't care whether you would accept it in a code review or
> not,


That's because we don't work in the same company, and I'm not
called upon to review your code. Code has to pass code review,
or be reworked until it does. It's as simple as that. And as
you've just been able to see, some people do feel strongly about
this issue, and it wouldn't surprise me if many places had such
rules in effect, and would not allow code which derived from a
standard container to pass review.

One thing is clear: using free functions rather than derivation
is a lot less likely to upset others who have to read your code
(unless they come from a Java background, and expect the C++ to
look like Java). IMHO, that's a very strong argument in favor
of free functions.

--
James Kanze
 
Reply With Quote
 
Steve Chow
Guest
Posts: n/a
 
      04-03-2010
On Apr 3, 7:23*am, Steve Chow <(E-Mail Removed)> wrote:
> Originally I had a bunch of related functions that all took a vector
> of Point2D as their argument.
> Point2D findGreatestDistance(std::vector<Point2D>& points);
>
> However, this didn't strike me as a very C++/OO way to do things, so I
> found a solution I was happy with in:
> class Path : public std::vector<Point2D>
> {
> public:
> * *Path();
> ~Path();
> * *Point2D findGreatestDistance();
> * /* related functions */
>
> };

So in review:
a.) Leave it floating free. Perhaps put the class & free floating
function in the same namespace?
b.) Something like
class Path
{
public:
Path();
~Path();
Point2D furthestPoint();
void add(Point2D& p) // purely an example
{
_points.push_back(p);
}
private:
std::vector<Point2D> _points;
/* related functions */

};
c.) free floating + generic (so it can take anything with an
iterator?)
 
Reply With Quote
 
Keith H Duggar
Guest
Posts: n/a
 
      04-03-2010
On Apr 3, 5:35 pm, James Kanze <(E-Mail Removed)> wrote:
> On Apr 3, 7:20 pm, "Alf P. Steinbach" <(E-Mail Removed)> wrote:
>
> > * Leigh Johnston:

>
> [...]
>
> > > 3. A standard library container destructor is not virtual so
> > > you cannot delete the associated object via a pointer to the
> > > container (base class).
> > >http://www.i42.co.uk/stuff/mutable_set.htm

> > Well, the most important one is still missing:
> > 0. Is the class derivation an implementation detail?
> > If so then exposing it as public, even as a public member instead of
> > as a base class,
> > 0.A. Introduces needless ways that bugs can creep in. E.g., when deriving
> > from std::vector<T>, then client code may use iterators incorrectly.

>
> In what way? IIUC, the argument is that the base abstraction
> *is* std::vector<Point2D>.


From the context it is hard (at least for me) to know what the
OPs base abstraction actually is. For example, it seems clear
that a "Path" is a sequence of points, but a contiguous one?
That does seems like an implementation detail. It seems almost
inevitable that sometime soon we are going to find that other
implementations (list, set) are more appropriate.

However, since vector has been exposed as a public interface
clients are likely to take advantage of that. Ie they are going
to write code that works (or works well) only with random
access iterators, contiguous memory, constant time size, etc.

> In that case, all that the
> derivation does is introduce a few extra convenience functions.
> And while the idiomatic way of doing this in C++ is by means of
> free functions, rather than derivation, I don't see how using
> derivation for this can possibly lead to something like misuse
> of the iterators.


See above. One example that comes to mind is assuming the
iterators are random access. Another more insidious one would
happen if he had chosen std::list for the first round and
then switched to std::vector. In that case clients might have
taken advantage of the invalidation semantics of std::list for
example to assume that an erase from the middle would not
invalidate any other iterators besides the one erased. Then
when he switches the implementation to std::vector, bang!
Hard to find memory corruptions start to creep in.

KHD
 
Reply With Quote
 
Steve Chow
Guest
Posts: n/a
 
      04-04-2010
> From the context it is hard (at least for me) to know what the
> OPs base abstraction actually is. For example, it seems clear
> that a "Path" is a sequence of points, but a contiguous one?

Mostly. As I explained in another post the sequence initially messed
up when dealing with
borders so I need to resort them. That's an unrelated issue that I've
cleared up though.
 
Reply With Quote
 
Daniel Pitts
Guest
Posts: n/a
 
      04-04-2010
On 4/3/2010 7:55 AM, Steve Chow wrote:
> On Apr 3, 7:39 am, "Alf P. Steinbach"<(E-Mail Removed)> wrote:
>> Instead, replace inheritance of std::vector<Point2D> with a private member.

>
>
>> Also, I'd rename 'findGreatestDistance' to just 'greatestDistance' (like,
>> although not in your code, I'd also rename 'calculateSin' to just 'sin', and so
>> on), and I'd make a type distinction between points and vectors, but as opposed
>> to getting rid of that inheritance this is to some degreee personal preference.
>>
>> Cheers& hth.,
>>
>> - Alf

>
> The reason I avoided that was because I'd have to have to write a
> public push_back function just to push_back to the private vector when
> I wanted to add a point, no?

That's like saying "The reason I avoided putting a roof on this room is
that I'd have to provide a light-fixture when I want to light the room. "

Yes, you sometimes have to write code to "unhide" a subset of existing
code. You still should do it. You're path is *not* a vector of
points, it is implemented in terms of one.

> I mean, it's trivial, but it just seemedlike a duplication.

It is not duplication, but delegation.

> But if it's more correct I'm not really going to
> argue. I tend to opt for the easy route a lot but have learned the
> consequence of doing that a lot is a nightmarish maintenance
> scenario.


Besides the poor semantics of that design, there is also risk in
deriving from a class which was not designed for such use (for example,
a class which does not have a virtual destructor).

That's right, classes have to be *designed* to be inheritable. It is an
all to common mistake (even by experienced programmers) to expect that
they can extend any class they choose. I personally only realized this
a few years ago when reading a Java concurrency book, and I've been
programming for nearly 20 years, almost 2/3rds of my life!

--
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
 
Reply With Quote
 
Keith H Duggar
Guest
Posts: n/a
 
      04-04-2010
On Apr 3, 5:56 pm, "Leigh Johnston" <(E-Mail Removed)> wrote:
> "James Kanze" <(E-Mail Removed)> wrote in message
> > One thing is clear: using free functions rather than derivation
> > is a lot less likely to upset others who have to read your code
> > (unless they come from a Java background, and expect the C++ to
> > look like Java). IMHO, that's a very strong argument in favor
> > of free functions.

>
> Your fondness of free functions probably stems from the fact that you used
> to be a C programmer (joke) or that <algorithm> is full of them which is
> fine as those functions are generic whereas you can argue that an augmented
> operator[] for std::vector is specific to that container and so quite
> rightly should be part of a class augmenting std::vector especially as it
> comes with state that is not dependent on the vector's state (so LSP still
> holds).
>
> So for a vector which supports 1-based indexing (instead of 0-based):
>
> int f = bounded_vector[10];
>
> is an improvement on than:
>
> int f = access_bounded_vector(v, 1, 10);
>
> This example simply doesn't work as a free function as what could be state
> in a class (for the value 1 above) now has to be passed to the stateless
> free function.


Your lack of imagination and inability to provide elegant free
function solutions probably stems from your OOP blinders. They
prevent you from grasping simple concepts such as overloading.
For example, the free function version of accessing a 1-based
vector type would not look like the unimaginative and verbose
junk you coded above, rather it would simply be

access(v,10)

just as it would be for 0-based vectors.

KHD
 
Reply With Quote
 
Steve Chow
Guest
Posts: n/a
 
      04-04-2010
On Apr 3, 12:33*pm, Stuart Golodetz
<(E-Mail Removed)> wrote:
> Well -- in the case of the imaging example, I can't see why either class
> would inherit from the other (an Image isn't a DataExtractor, or
> vice-versa). I'm assuming the DataExtractor class is some sort of
> functor, incidentally. The relationship seems to be that some other code
> instantiates a DataExtractor, and uses it to extract data from an image.
> Or something along those lines Either way, doesn't sound like there
> should be an inheritance relationship between those two classes.

The reason it's I said it was murky was the dataExtractor class is
accessing (and) requires virtually all the private variables of the
image class. I have a get/set method for pretty much everything and
I'm using them everywhere. I mean, I have a flood method in
dataExtractor which get's a pointer to the pixeldata in image, gets
the size of the image, and performs a flood fill operation. I do this
to mark it visually and logically as already processed. But I also
obtain scanlines of the blob, create a binary duplication of the
flooded area, etc. The only way I can obtain this info is through a
flood operation. But flood fill? Flood sounds like it should be in the
image class, or maybe an image tools class, but it also does all this
specialized stuff that doesn't mean anything to any other program or
class.

I sometimes wish I had a formal education in this stuff. I'm self-
taught and I think it shows.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
integer >= 1 == True and integer.0 == False is bad, bad, bad!!! rantingrick Python 44 07-13-2010 06:33 PM
Bad media, bad files or bad Nero? John Computer Information 23 01-08-2008 09:17 PM
ActiveX apologetic Larry Seltzer... "Sun paid for malicious ActiveX code, and Firefox is bad, bad bad baad. please use ActiveX, it's secure and nice!" (ok, the last part is irony on my part) fernando.cassia@gmail.com Java 0 04-16-2005 10:05 PM
24 Season 3 Bad Bad Bad (Spoiler) nospam@nospam.com DVD Video 12 02-23-2005 03:28 AM
24 Season 3 Bad Bad Bad (Spoiler) nospam@nospam.com DVD Video 0 02-19-2005 01:10 AM



Advertisments