In a c++98 project I have a class Items
that wraps a std::map
:
#include <string>
#include <map>
class Item { /* ... */ }; // Some class holding some data
class Items
{
public:
typedef std::map<std::string,Item> container_type;
typedef container_type::iterator iterator;
Items() {}
iterator find(const String& k) { return i_map.find(k); }
iterator end() { return i_map.end(); }
void erase(iterator i) { i_map.erase(i); }
// ...
private:
container_type i_map;
};
Its main use is to search a certain Item
and if found use and remove it.
The first obvious API I proposed to consume an item was this:
Items items;
Items::iterator i = items.find("some-id");
if( i!=items.end() )
{
const Item& item = i->second; // Get and use the item
items.erase(i); // Item consumed: remove it from items
}
...But I was asked to hide the concepts of iterator
and pair
from the class API.
To meet this new requirement the first idea was to store internally an iterator i_found
to remember the last found item:
#include <stdexcept>
#include <string>
#include <map>
class Item { /* ... */ }; // Some class holding some data
class Items
{
public:
typedef std::map<std::string,Item> container_type;
typedef container_type::iterator iterator;
Items() : i_found( i_map.end() ) {}
#define should_be_const // const // Unfortunately 'i_found' must be a non const 'iterator' in order to be erased
bool contains(const std::string& k) should_be_const
{
i_found = i_map.find(k);
return i_found!=i_map.end();
}
const Item& get(const std::string& k) should_be_const
{
if(i_found==i_map.end() || k!=i_found->first) i_found = i_map.find(k); // search if necessary
if(i_found!=i_map.end()) return i_found->second;
else throw std::runtime_error("key \'" + std::string(k) + "\' not found!");
}
void erase_found()
{
i_map.erase(i_found);
i_found = i_map.end(); // invalidate last find
}
private:
container_type i_map;
mutable iterator i_found; // Last found item
};
This gives the possibility to write:
Items items;
if( items.contains("some-id") )
{
const Item& item = items.get("some-id"); // Get and use the item
items.erase_found(); // Item used: remove it from items
}
I know that if this is an improvement or not is debatable, I'm not asking about that (yeah, I don't like it either).
Is there a way in this last implementation to make the methods contains()
and get()
const
?
Given the requirement above, I was also interested in suggestions about different approaches.
While Item
copy construction is acceptable, I'd like to to avoid to construct an item
if "some-id" was not found, as in this alternative I was desperately trying:
bool Items::extract_if_present(const std::string& k, Item& item)
{
iterator i = i_map.find(k);
if( i != i_map.end() )
{
item = i->second;
i_map.erase(i);
return true;
}
return false;
}
Item item; // <-- Avoidable useless work if not found
if( items.extract_if_present("some-id", item) )
{
//item; // Use the copied item
}
With optional
(not std, which is C++17, but could be done in C++98 (using boost
for example)), then you might have in interface
optional<Item> get(const std::string&) const;
optional<Item> extract(const std::string&);
With usage similar to:
Items items;
// ...
if (optional<Item> item = items.extract("some-id"))
{
// use *item;
}