patterncppModerate
Is my C++11 generic container a good design?
Viewed 0 times
genericdesigncontainergood
Problem
After refactors and refactors and the discovery of very common patterns on many of the classes of the software I wrote, I decided that it would be fine to have something like an arbitrary-keyed map, that would require the key to be stored in the object (which allows to use the object's name as a key for instance).
I would like to know if it is a good design, and what are possible pitfalls I didn't encounter and might in the future (and how to prevent them).
It is far from perfect (no optimisations, not /that/ generic...) but allowed me to have cleaner code.
The container code:
```
#pragma once
#include
#include
#include
#include
template
using SimpleVec = std::vector>;
template class Container = SimpleVec,
class String = std::string>
class Iterable
{
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
begin(Iterable& i);
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
end(Iterable& i);
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
cbegin(const Iterable& i);
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
cend(const Iterable& i);
public:
Iterable() = default;
Iterable(const Iterable& c) = delete;
Iterable& operator=(const Iterable& c) = delete;
virtual ~Iterable() = default;
template
void performUniquenessCheck(const K&... args)
{
if(std::any_of(begin(*this),
end(*this),
T::hasSame(args...)))
{
throw "Element already exists";
}
}
template
T& create(K&&... args)
{
element
I would like to know if it is a good design, and what are possible pitfalls I didn't encounter and might in the future (and how to prevent them).
It is far from perfect (no optimisations, not /that/ generic...) but allowed me to have cleaner code.
The container code:
```
#pragma once
#include
#include
#include
#include
template
using SimpleVec = std::vector>;
template class Container = SimpleVec,
class String = std::string>
class Iterable
{
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
begin(Iterable& i);
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
end(Iterable& i);
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
cbegin(const Iterable& i);
template class friendContainer,
class friendString>
friend typename friendContainer::iterator
cend(const Iterable& i);
public:
Iterable() = default;
Iterable(const Iterable& c) = delete;
Iterable& operator=(const Iterable& c) = delete;
virtual ~Iterable() = default;
template
void performUniquenessCheck(const K&... args)
{
if(std::any_of(begin(*this),
end(*this),
T::hasSame(args...)))
{
throw "Element already exists";
}
}
template
T& create(K&&... args)
{
element
Solution
[...]I decided that it would be fine to have something like an arbitrary-keyed
map, that would require the key to be stored in the object (which allows to use
the object's name as a key for instance).
To restate your problem: you want the add behavior of a set (i.e., you add the object to the collection without any other information), but you want the retrieve behavior of a map (i.e., you retrieve the object by a key, not by a reference to that object). Additionally, from the code, it seems that you want to be able to retrieve an object by multiple keys (e.g., by both ID # and name), all of which then need to be unique.
First, it's probably a bad idea to have objects being keyed in more than one way in the same container. It effectively means you have aliases for the same object, and that can get quite confusing to manage. It also means your lookups are pretty much stuck at \$O(N)\$ (instead of \$O(log(N))\$ or \$O(1)\$). Why? Because faster than \$O(N)\$ lookups require that the data either be sorted (so an \$O(log(N))\$ search can be used), or hashed (so that an \$O(1)\$ probe can be performed). Using your example, "name" and "Id" fields won't both be such that sorting one will cause the other to be sorted, nor will their hash values come out identically. Thus, retrieving items from the container degenerates to \$O(N)\$ when doing lookups using anything other than the primary key.
This isn't entirely true -- with one map, you could jury-rig a string key with namespaces, and in that manner have multiple keys without a slowdown of the map (although the hash calculation time might increase as the strings get longer). More reasonably, you could have one map per key. I have an aggregate_automap proof-of concept that takes this approach. It's templated, and it allows an arbitrary number of maps (each based on an automap translator) to be added to, removed from, and (by selecting a specific map from the group) searched in. However, it's too big and too ugly for me to, in good conscience, put up onto CR at the moment. Know that it -is- possible, if you absolutely need to look up objects using multiple keys... but try to only use multiple keys as an absolute last resort. :)
What I would recommend is a wrapper around
```
#include
#include
#include
#include
// A class that has data you want to construct automatic keys with.
// This class can be any existing class you like. It does not need to be
// modified to support auto-keying.
class MyClass
{
public:
MyClass(const std::string& name, long long ssn) : m_name(name), m_ssn(ssn) {}
const std::string& getName() const { return m_name; }
long long getSsn() const { return m_ssn; }
private:
std::string m_name;
long long m_ssn;
};
// A general template that concrete AutomapTranslator classes can be based on.
template , typename EqualTo = std::equal_to>
class AutomapTranslator
{
public:
// The type of object the keys will map to.
typedef Value mapped_type;
// The type of the key
typedef const Key key_type;
// Custom hash and equal_to functions, if your Key does not have these
// provided by default.
typedef Hash hash;
typedef EqualTo equal_to;
};
// A class that implements the logic for automatically generating keys from
// instances of a class.
// You will need to implement at least one of these for each class you want to
// auto-key. You might implement more than one of these for a single class, if
// you want to have different ways of auto-keying the class (e.g., if you want
// to be unique by SSN instead of name, or force both SSN and name into the
// key, or have a key based on a long long instead of a string...)
class MyClassAutomapTranslator : public AutomapTranslator
{
public:
// For each constructor signature in mapped_type, you will need a
// matching getKey function signature (for "emplace").
static key_type getKey(const std::string& name, long long ssn) { return name; }
// At minimum, you will also need a getKey that converts an
// arbitrary mapped_type to key_type.
static key_type getKey(const mapped_type& val) { return val.getName(); }
private:
};
// An extension of unordered_map, with functions that allow a translator object
// to auto-generate keys when inserting key-less values.
template
class unordered_automap :
public std::unordered_map
{
// For convenience/brevity... :)
typedef std::unordered_map map_type;
public:
template
std:
map, that would require the key to be stored in the object (which allows to use
the object's name as a key for instance).
To restate your problem: you want the add behavior of a set (i.e., you add the object to the collection without any other information), but you want the retrieve behavior of a map (i.e., you retrieve the object by a key, not by a reference to that object). Additionally, from the code, it seems that you want to be able to retrieve an object by multiple keys (e.g., by both ID # and name), all of which then need to be unique.
First, it's probably a bad idea to have objects being keyed in more than one way in the same container. It effectively means you have aliases for the same object, and that can get quite confusing to manage. It also means your lookups are pretty much stuck at \$O(N)\$ (instead of \$O(log(N))\$ or \$O(1)\$). Why? Because faster than \$O(N)\$ lookups require that the data either be sorted (so an \$O(log(N))\$ search can be used), or hashed (so that an \$O(1)\$ probe can be performed). Using your example, "name" and "Id" fields won't both be such that sorting one will cause the other to be sorted, nor will their hash values come out identically. Thus, retrieving items from the container degenerates to \$O(N)\$ when doing lookups using anything other than the primary key.
This isn't entirely true -- with one map, you could jury-rig a string key with namespaces, and in that manner have multiple keys without a slowdown of the map (although the hash calculation time might increase as the strings get longer). More reasonably, you could have one map per key. I have an aggregate_automap proof-of concept that takes this approach. It's templated, and it allows an arbitrary number of maps (each based on an automap translator) to be added to, removed from, and (by selecting a specific map from the group) searched in. However, it's too big and too ugly for me to, in good conscience, put up onto CR at the moment. Know that it -is- possible, if you absolutely need to look up objects using multiple keys... but try to only use multiple keys as an absolute last resort. :)
What I would recommend is a wrapper around
unordered_set, such that neither your class-to-be-contained nor your container have to be modified to deal with the special details of your auto-keying. I have an example of such a solution below, but due to the limitations of search speed (which I mentioned earlier), it does not implement multi-keying, nor does it implement your getFamily functionality. However, both of those pieces of functionality can be implemented in \$O(N)\$ time using iterator traversal logic similar to what you already have.```
#include
#include
#include
#include
// A class that has data you want to construct automatic keys with.
// This class can be any existing class you like. It does not need to be
// modified to support auto-keying.
class MyClass
{
public:
MyClass(const std::string& name, long long ssn) : m_name(name), m_ssn(ssn) {}
const std::string& getName() const { return m_name; }
long long getSsn() const { return m_ssn; }
private:
std::string m_name;
long long m_ssn;
};
// A general template that concrete AutomapTranslator classes can be based on.
template , typename EqualTo = std::equal_to>
class AutomapTranslator
{
public:
// The type of object the keys will map to.
typedef Value mapped_type;
// The type of the key
typedef const Key key_type;
// Custom hash and equal_to functions, if your Key does not have these
// provided by default.
typedef Hash hash;
typedef EqualTo equal_to;
};
// A class that implements the logic for automatically generating keys from
// instances of a class.
// You will need to implement at least one of these for each class you want to
// auto-key. You might implement more than one of these for a single class, if
// you want to have different ways of auto-keying the class (e.g., if you want
// to be unique by SSN instead of name, or force both SSN and name into the
// key, or have a key based on a long long instead of a string...)
class MyClassAutomapTranslator : public AutomapTranslator
{
public:
// For each constructor signature in mapped_type, you will need a
// matching getKey function signature (for "emplace").
static key_type getKey(const std::string& name, long long ssn) { return name; }
// At minimum, you will also need a getKey that converts an
// arbitrary mapped_type to key_type.
static key_type getKey(const mapped_type& val) { return val.getName(); }
private:
};
// An extension of unordered_map, with functions that allow a translator object
// to auto-generate keys when inserting key-less values.
template
class unordered_automap :
public std::unordered_map
{
// For convenience/brevity... :)
typedef std::unordered_map map_type;
public:
template
std:
Code Snippets
#include <cstdlib>
#include <iostream>
#include <string>
#include <unordered_map>
// A class that has data you want to construct automatic keys with.
// This class can be any existing class you like. It does not need to be
// modified to support auto-keying.
class MyClass
{
public:
MyClass(const std::string& name, long long ssn) : m_name(name), m_ssn(ssn) {}
const std::string& getName() const { return m_name; }
long long getSsn() const { return m_ssn; }
private:
std::string m_name;
long long m_ssn;
};
// A general template that concrete AutomapTranslator classes can be based on.
template <typename Key, typename Value, typename Hash = std::hash<Key>, typename EqualTo = std::equal_to<Key>>
class AutomapTranslator
{
public:
// The type of object the keys will map to.
typedef Value mapped_type;
// The type of the key
typedef const Key key_type;
// Custom hash and equal_to functions, if your Key does not have these
// provided by default.
typedef Hash hash;
typedef EqualTo equal_to;
};
// A class that implements the logic for automatically generating keys from
// instances of a class.
// You will need to implement at least one of these for each class you want to
// auto-key. You might implement more than one of these for a single class, if
// you want to have different ways of auto-keying the class (e.g., if you want
// to be unique by SSN instead of name, or force both SSN and name into the
// key, or have a key based on a long long instead of a string...)
class MyClassAutomapTranslator : public AutomapTranslator<std::string, MyClass>
{
public:
// For each constructor signature in mapped_type, you will need a
// matching getKey function signature (for "emplace").
static key_type getKey(const std::string& name, long long ssn) { return name; }
// At minimum, you will also need a getKey that converts an
// arbitrary mapped_type to key_type.
static key_type getKey(const mapped_type& val) { return val.getName(); }
private:
};
// An extension of unordered_map, with functions that allow a translator object
// to auto-generate keys when inserting key-less values.
template <typename T>
class unordered_automap :
public std::unordered_map<
typename T::key_type,
typename T::mapped_type,
typename T::hash,
typename T::equal_to>
{
// For convenience/brevity... :)
typedef std::unordered_map<
typename T::key_type,
typename T::mapped_type,
typename T::hash,
typename T::equal_to> map_type;
public:
template<typename... Args>
std::pair<typename map_type::iterator, bool> autoemplace(Args&&... args)
{
return this->emplace(T::getKey(args...), args...);
}
template<typename... Args>
typename map_type::iterator autoemplace_hint(typename map_type::const_iterator position, Args&&... args)
{
Context
StackExchange Code Review Q#47491, answer score: 12
Revisions (0)
No revisions yet.