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

Question

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?

A subscript overload makes sense for associate types like `std::map` and `std::unordered_map` because it mimics the behavior of an array, just using an arbitrary key type instead of an integer index. But sets don't hold key/value pairs, they just hold keys. How is `someset["a"]` supposed to work? You can't assign to it because there's no value associated with that key. What it's supposed to return? Again, no value... It doesn't make sense. Thus, no subscript overloads. — Shawn, Sep 09 '18 at 06:14
This is a `set`, right? That means items in the container do not have a *fixed* index. That is, `set[2]` may become a different object simply by inserting an element into the set. So why exactly do you need to index a set by numeric indices like this? — Nicol Bolas, Sep 09 '18 at 06:15
Note that the container classes in C++ are not designed to be publicly inherited from anyway. If you really want to use such a special set container (which is a bad idea to begin with, see all the answers), then you should privately inherit from `std::set` or use a data member to which you delegate all required operations. — Christian Hackl, Sep 09 '18 at 06:16
@ChristianHackl: `std::stack` and `std::queue`, with protected members, are clearly designed to be inherited from. I know there is a misguided notion floating around that public inheritance is contra-indicated when there is not a virtual destructor. Perhaps that's what you're referring to? — Cheers and hth. - Alf, Sep 09 '18 at 06:21
@Cheersandhth.-Alf: But you don't need public inheritance to access them. — Christian Hackl, Sep 09 '18 at 06:27
@Cheersandhth.-Alf: Yes, I'm refering to the "misguided" notion that classes without virtual functions are usually not meant to be publicly inherited from. — Christian Hackl, Sep 09 '18 at 06:30
One can't (rationally) infer intent from such features. And even where there is a known intent, it should have no bearing on one's technical decisions. Unless one is hopelessly mired in authority argumentation. — Cheers and hth. - Alf, Sep 09 '18 at 06:49
@Cheersandhth.-Alf: Now you are playing the devil's advocate. — Christian Hackl, Sep 09 '18 at 07:38

score 10 · Accepted Answer · answered Sep 09 '18 at 06:04

10

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.

answered Sep 09 '18 at 06:04

Cheers and hth. - Alf

142,714
15
209
331

2

"*Indexing should never be more than logarithmic time, that's what's expected.*" Actually, what's expected is *constant time*. The Range TS even codifies this by declaring that Random Access Ranges provide `operator[]` support, that this function be O(1). – Nicol Bolas Sep 09 '18 at 06:11
5

@NicolBolas: Those expectations are not incompatible. In some cases it can be reasonable with a logarithmic time indexing operator. The most common example is the indexing operator of `std::map`. – Cheers and hth. - Alf Sep 09 '18 at 06:13

score 4 · Answer 2 · answered Sep 09 '18 at 06:06

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.

score 0 · Answer 3 · answered Sep 09 '18 at 06:47

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).

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

3 Answers3