Are there any reasons not to extend std::set to add a subscript operator?

403 views Asked by At

I'm using std::set to store unique instances of a class. std::set does not have an overloaded subscript operator so you can't do set[0] for example.

I found a way to do it:

auto myClass = *std::next(set.begin(), index);

However, I found it monotonous to copy that code over and over again. So I decided it would be much more convenient to just extend std::set (class sset) and just overload the subscript operator in it.

template <class T>

class sset: public std::set<T>
{
public:
    T operator[](const uint32_t i) const
    {
        if(i <= (this->size()-1))
            return *std::next(this->begin(), i);
        else
            throw std::out_of_range("Index is out of range");
    }
};

int main()
{
    auto myClass = set[0]; //works and no exception thrown

    return 0;
}

I achieved the desired behavior but it occurred to me that there must have been a reason the standard didn't include a subscript operator. Surely not just laziness.

Is there any preceived disadvantage or possible future problems one can see doing this?

3

There are 3 answers

2
Cheers and hth. - Alf On BEST ANSWER

Indexing should never be more than logarithmic time, that's what's expected. This indexing is (at least) linear time. That's awfully inefficient. If you run through all items in a set, using that indexing, you get quadratic total time. That's a good reason to not do it.


For the code shown, note that

if(i <= (this->size()-1)

doesn't handle size 0 well. In this case you get unsigned wrap-around so that the condition is true. Dereferencing the end iterator is then Undefined Behavior.

0
Michael J On

The std::set doesn't generally have an efficient way to access the nth element. The method you are using will start at the beginning of the set and advance one element at a time until it gets to the nth. This would be very slow for large sets.

If you need it, then by all means do it, but`be aware of the inefficiency.

0
Aconcagua On

Apart from efficiency issues already mentioned, std::set (as most of the other standard containers) is not designed to be inherited from - especially, it does not provide a virtual destructor, so the following is bound to fail:

std::set<MyType>* s = new sset<MyType>();
delete s;

Sure, there should be little reasons for having a set created this way, but the issue still remains...

If you really, really need nth element and don't want to rewrite your sample code all the time, I'd rather have a separate function for than (at least) questionable inheritance:

template <typename T>
T& at(std::set<T>& s, size_t index)
{
    if(i < s.size())
        return *std::next(s.begin(), index);
    throw std::out_of_range("index is out of range");
}
template <typename T>
T const& at(std::set<T> const& s, size_t index)
{
    if(i < s.size())
        return *std::next(s.begin(), index);
    throw std::out_of_range("index is out of range");
}

Don't ever use that in a for loop iterating from 0 to size, use iterators instead or preferably a range based loop (which actually maps to a loop using iterators).