5

While working on some graphics code a while back, I wrote Rect and Region classes using ints as the underlying coordinate holder, and that worked fine. The Region was implemented as a simple class extension to an STL list, and just contains a list of Rects.

Now I also need the same kinds of classes using doubles as the underlying coordinate holder, and decided to try my hand at templatizing it. So I basically replaced "int" with "typename T" in an intelligent manner and fixed the problems.

But there's one remaining problem that has me stumped. I want to calculate a Region's bounding box by doing a union on all the Rects that comprise it. That works fine when not templatized, but g++ chokes on the list iterator when this is templatized.

Here's the relevant code:

// Rect class that always remains normalized
template <typename T>
class KRect
{
public:

    // Ctors
    KRect(void)
        : _l(0), _t(0), _r(0), _b(0)
    {
    }
    void unionRect(const KRect& r)
    {
        ...
    }

private:
    T _l, _t, _r, _b;
};

// Region class - this is very brain-dead
template <typename T>
class KRegion : public std::list< KRect<T> >
{
public:
    ...

    // Accessors
    KRect<T> boundingBox(void)
    {
        KRect<T> r;
        iterator i;
        for (i = this->begin(); i != this->end(); i++)
        {
            r.unionRect(*i);
        }
        return r;
    }
    ...
};

When that code isn't part of a template, so that T is definite (e.g. an int), the "iterator i" line works fine. But in what you see above, g++ on Ubuntu emits errors which I don't find very informative:

include/KGraphicsUtils.h: In member function ‘KRect<T> KRegion<T>::boundingBox()’:
include/KGraphicsUtils.h:196: error: expected ‘;’ before ‘i’
include/KGraphicsUtils.h:197: error: ‘i’ was not declared in this scope
include/KGraphicsUtils.h: In member function ‘KRect<T> KRegion<T>::boundingBox() [with T = int]’:
--- redacted ---:111:   instantiated from here
include/KGraphicsUtils.h:196: error: dependent-name ‘std::foo::iterator’ is parsed as a non-type, but instantiation yields a type
include/KGraphicsUtils.h:196: note: say ‘typename std::foo::iterator’ if a type is meant

My guess is this is a type qualification issue with some template-y spin I'm not familiar with. I've tried all kinds of things like:

std::list< KRect<T> >::iterator i;
this->iterator i;

but nothing seems to work.

Any suggestions?

outis
  • 75,655
  • 22
  • 151
  • 221
Bob Murphy
  • 5,814
  • 2
  • 32
  • 35
  • Which compiler are you using? I am able to compile it on VC++2010. – Jagannath Feb 08 '10 at 01:45
  • He already said g++ - VC++ is traditionally very lax regarding dependent names without `typename` or `template`. – Georg Fritzsche Feb 08 '10 at 01:47
  • You should not inherit from a STL container, ever, because they have not been thought to be polymorphic (ie no virtual destructor). The proper way is to use composition, and write forward methods appropriately... though it's tedious. – Matthieu M. Feb 08 '10 at 10:10
  • Agreed - but this is a proof-of-concept implementation that has to be rapidly written, so some hackish shortcuts are acceptable. – Bob Murphy Feb 09 '10 at 18:45

2 Answers2

9

iterator is a dependent type (it depends on a template argument) and needs to be prefixed with typename:

typename std::list< KRect<T> >::iterator i;

Better style would be to provide a class-wide typedef:

template <typename T>
class KRegion : public std::list< KRect<T> >
{
    typedef std::list< KRect<T> > base;
    typedef typename base::iterator iterator;
    // ...
};
Georg Fritzsche
  • 97,545
  • 26
  • 194
  • 236
3

I think gf has your answer, but I'd like to suggest having the region manage a list as a member instead of a base class:

template <typename T>
class KRegion
{
protected:
     typedef std::list< KRect<T> > ListType;
     ListType list;
public:
    ...
    // Accessors
    void addRect(KRect<T> & rect) { list->push_back(rect); }
    ...
    KRect<T> boundingBox(void)
    {
        KRect<T> r;
        ListType::iterator i;
        for (i = list->begin(); i != list->end(); i++)
        {
            r.unionRect(*i);
        }
        return r;
    }
    ...
};

My motivation for this suggestion is that you may, one day, want to use a different container for storing your KRects, and having the list as an internal member would let you do so without breaking all of your client code.

Community
  • 1
  • 1
e.James
  • 116,942
  • 41
  • 177
  • 214
  • e.James, you also are a gentleman and a scholar! That's a great idea; however, right now I want clients to have direct access to list members because I'm too lazy to write all the accessors. :-) – Bob Murphy Feb 08 '10 at 02:14
  • That's entirely fair `:)` We've all been there. – e.James Feb 08 '10 at 04:28
  • Just hope no client ever write `ListType* l = new KRegion(); delete l;`... – Matthieu M. Feb 08 '10 at 10:11