0

I have a very special scenario, and I'm trying to add functionality to "list"...

#include <list>

template <typename T>
class ShortList : public std::list<T> {
  private:
    unsigned short max_items;

  public:
    // Getter and setter methods
    unsigned short getMaxItems (void) {
        return this.max_items;
    }
    void setMaxItems (unsigned short max_items) {
        this.max_items = max_items;
        return;
    }

    // Work methods
    void insertItemSorted (T item) {
        std::list<T>::iterator i, e = this.short_list.end();

        // Insertion sort O(n*max_items)
        for ( i = this.short_list.begin() ; i != e ; ++i ) {
            // Insert if smaller
            if ( item.getDiff() < (*i).getDiff() ) {
                this.short_list.insert(i, item);
                // Restrict list size to max items
                if ( this.short_list.size() > this.max_items ) {
                    this.short_list.erase(this.short_list.end());
                }
            }
        }
        return;
    }
}

I can't understand what I'm doing wrong here. Here are my compile errors...

ShortList.cpp: In member function 'int ShortList<T>::insertItemSorted(T)':
ShortList.cpp:21: error: expected `;' before 'i'
ShortList.cpp:24: error: 'i' was not declared in this scope
ShortList.cpp:24: error: 'e' was not declared in this scope
ShortList.cpp: At global scope:
ShortList.cpp:35: error: expected unqualified-id at end of input

I feel as though I'm following a C++ tutorial to the letter. Can anyone explain where I've gone wrong? I know it's poor form to extend the functionality of a STL container, but this a is purely a scholarly pursuit.

Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
Zak
  • 12,213
  • 21
  • 59
  • 105
  • 1
    It's not just "poor form" to inherit from STL containers, it's a terrifically bad idea because they are not designed for it. For starters, many of them do not have virtual destructors. – Cody Gray - on strike Feb 03 '12 at 05:12
  • You should implement a wrapper for std::list instead. – twsaef Feb 03 '12 at 05:15
  • @CodyGray, does it mean that if any class doesn't have `virtual` function/destructor should never get publicly derived ? Apart from your comment, many a times I have come across this matter that STL containers should never get derived. I feel that, it's an unnecessary fear. If someone wants to extend them for functionality extension, there shouldn't be any problem – iammilind Feb 03 '12 at 05:23
  • @iam: No, not necessarily. But it is certainly a concern, and that's only the tip of the iceberg. In general, you should only inherit from types that have been specifically designed to be inherited from. I went looking for questions about this, as there are lots of good answers already, but I find [you already asked one](http://stackoverflow.com/questions/6806173). – Cody Gray - on strike Feb 03 '12 at 05:26
  • Also see: [Is there any real risk to deriving from the C++ STL containers?](http://stackoverflow.com/questions/922248), [Thou shalt not inherit from std::vector](http://stackoverflow.com/questions/4353203), [Is it okay to inherit implementation from STL containers, rather than delegate?](http://stackoverflow.com/questions/2034916/), [Advice on a better way to extend C++ STL container with user-defined methods](http://stackoverflow.com/questions/679520/), [Can/Should i inherit from STL iterator?](http://stackoverflow.com/questions/6471019/) – Cody Gray - on strike Feb 03 '12 at 05:27
  • @CodyGray, yes I do remember; that's why I pointed out. I haven't accepted any answer in that question yet :). I don't say that the fear of extending STLs is wrong, but rather the fear is overestimated. – iammilind Feb 03 '12 at 05:30
  • @iam: Lots of fears are. The way I look at it, it's just not worth the risk. Composition is just as effective as inheritance (and probably clearer) in this case, and it avoids the potential snags. If I was the only developer that would ever have to maintain my code (and I could be assured that future me would never make a mistake and write code that assumed standard polymorphic behavior) then maybe it would be okay. But since that assumption is never realistic, and there's really no harm to doing it the "right" way, I don't think there's any good justification for inheritance. – Cody Gray - on strike Feb 03 '12 at 05:32
  • Composition would avoid one pitfall: inheriting methods that don't respect class invariants. E.g this ShortList inherits `insert` which allows you to make longer and unsorted lists. (Private inheritance would avoid that too, forcing you to choose which methods of the base class you make available.) – visitor Feb 03 '12 at 09:59

1 Answers1

4

I think the immediate cause of your error is that you need typename:

typename std::list<T>::iterator i, e = this.short_list.end();

... because the compiler doesn't realize that iterator is a type yet.

But you really don't want to derive from std::list, and you don't want to write your own sort.

Fred Larson
  • 60,987
  • 18
  • 112
  • 174
  • 1
    I just need a specialized list for a very specific, short-term problem, otherwise I would never do this. – Zak Feb 03 '12 at 05:17
  • The typename keyword worked, now I still have the problem... ShortList.cpp:36: error: expected unqualified-id at end of input – Zak Feb 03 '12 at 05:20
  • 2
    @Zak: I think you're missing a semicolon on the end of your class declaration. – Fred Larson Feb 03 '12 at 05:23
  • 1
    Thanks @Fred Larson! You nailed it. I'll delete it when I'm finished (so no one's head explodes). Like I said, it's just for some testing! Thanks again everyone for your input! Z@K! – Zak Feb 03 '12 at 05:26