Duplicate value storage:
One disadvantage of your approach is that it stores a duplicate of each value inserted. The cost can become prohibitive for large objects, such as class instances stored by value.
A possible approach to avoid the duplicate would be just doing a linear search over the vector every time a new insertion takes place. For small arrays, this is usually quite fast.
Another possible implementation would be to keep the vector always sorted, then a fast binary search could be performed before insertions to ensure uniqueness.
Proper way of testing set/map
insertion:
This code doesn't seem correct:
bool success = true;
if(m_uset.insert(x)) m_vec.push_back(x);
else success = false;
return success;
The insert
method of sets and maps of the Standard Library will return you a pair consisting of an iterator to the inserted element (or to the element that prevented the insertion) and a bool denoting whether the insertion took place (from here).
So the correct way to test if the insertion was successful would be:
if (m_uset.insert(value).second == true)
{
...
}
max_size
is never checked:
You've stated in the title that the container should have a fixed maximum size, however, you never check to see if the limit size was reached. push_back
in a vector will resize the container if its initial capacity is exceeded. Same is true for set::insert
.
push()
is taking a mutable reference, which can cause confusion:
The signature of bool push(T& x)
, taking a mutable reference to T
, shows the called that this method has the intention or possibility of altering the input parameter. That would be a pretty strange behaviour for a container. When storing some data, you don't want it to be changed by the container. The proper signature here would be to pass the value by const reference:
bool push(const T& x);
That will prevent a copy and will pass a clear message to the caller that the input value is not changed, only stored.
As you polish this interface, you should consider move semantics (C++11), and add a move reference (T&&
) overload for push()
.
empty()
is a more idiomatic way of checking for empty containers:
The traditional way of testing if a Standard container is empty is by calling the empty()
method, as opposed to if (c.size() > 0)
.