principlecppModerate
Strategy pattern - design hash table
Viewed 0 times
designhashpatternstrategytable
Problem
My programming skill has gotten rusty, so I would like to elicit constructive criticism that can make me write code better. I know the standard design is to use templates, and even obtain the hash function, allocator, etc through it, but I wanted to experiment with patterns, so I'm pretty sure my design here blows. but I'm trying to use the strategy pattern to make a customizable hash table interface, just to reacquaint myself with C++.
How do I design this better (without using all the template parameters, for specifying hash function, etc)? If not the strategy pattern, what pattern would fit better?
```
//The generic map interface
template
class Map
{
private:
//Make a reference class so we can enable [] operation
class reference
{
public:
operator V const () { return map_.Find(key_); }
reference& operator=(const V& val) { map_.Add(key_, val); return *this; }
reference& operator=(const reference& ref) { this = (V&) ref; return this; }
private:
reference (Map& map, const K key) : map_(map), key_(key) {}
Map& map_;
K key_;
friend class Map;
};
public:
Map() : hash_(NULL), ds_(NULL), size_(0) { }
void SetHashStrategy(int hash_type = HashType::SIMPLE_HASH);
void SetCollisionStrategy(int collision_method = CollisionType::CHAINING);
void SetDataStructureStrategy(int data_structure_types = DSType::DYNAMIC_MAP);
virtual ~Map()
{
Empty();
delete hash_;
delete ds_;
}
//Interface functions
void Add(K key,V value) { ++size_; ds_->Add(key, value); }
void Remove(K key) { --size_; ds_->Remove(key); }
V& Find(K key) { return ds_->Find(key); }
size_t Size() { return size_; }
void Empty() { size_ = 0; ds_->Empty(); }
//Square bracket access
reference operator[] (const K& key) { return (reference(*this, key)); }
//printin
How do I design this better (without using all the template parameters, for specifying hash function, etc)? If not the strategy pattern, what pattern would fit better?
```
//The generic map interface
template
class Map
{
private:
//Make a reference class so we can enable [] operation
class reference
{
public:
operator V const () { return map_.Find(key_); }
reference& operator=(const V& val) { map_.Add(key_, val); return *this; }
reference& operator=(const reference& ref) { this = (V&) ref; return this; }
private:
reference (Map& map, const K key) : map_(map), key_(key) {}
Map& map_;
K key_;
friend class Map;
};
public:
Map() : hash_(NULL), ds_(NULL), size_(0) { }
void SetHashStrategy(int hash_type = HashType::SIMPLE_HASH);
void SetCollisionStrategy(int collision_method = CollisionType::CHAINING);
void SetDataStructureStrategy(int data_structure_types = DSType::DYNAMIC_MAP);
virtual ~Map()
{
Empty();
delete hash_;
delete ds_;
}
//Interface functions
void Add(K key,V value) { ++size_; ds_->Add(key, value); }
void Remove(K key) { --size_; ds_->Remove(key); }
V& Find(K key) { return ds_->Find(key); }
size_t Size() { return size_; }
void Empty() { size_ = 0; ds_->Empty(); }
//Square bracket access
reference operator[] (const K& key) { return (reference(*this, key)); }
//printin
Solution
You're misusing the term strategy pattern.
Instead of doing this kind of thing:
The strategy pattern would look something more like this:
That is, you would have a
with concrete implementations in subclasses,
for example
And then, the code to initialize the data structure would look like this:
where
In other words:
-
Your hashmap class shouldn't have to know how a strategy is implemented. At no point should the code have conditional statements on
-
The same goes for all the other kind of strategies that you implied: hash strategy, collision strategy. Define a common interface for each strategy that the hashmap wants to work with. Move the implementations of strategies outside of the hashmap, into subclasses of the common interfaces.
-
These strategies should not change during the lifetime of a hashmap. It wouldn't be efficient to switch the strategy of an existing hashmap. So it would be better to make these strategies required at construction time, and remove the setters.
Instead of doing this kind of thing:
template
void Map::SetDataStructureStrategy(int ds_type)
{
if(ds_ != NULL)
{
delete ds_;
}
if(ds_type == DSType::DYNAMIC_MAP)
{
ds_ = new DynamicMap;
}
else if(ds_type == DSType::ARRAY_MAP)
{
ds_ = new ArrayMap;
}
}The strategy pattern would look something more like this:
template
void Map::SetDataStructureStrategy(DataStructureStrategy * dsStrategy)
{
this->dsStrategy = dsStrategy;
}That is, you would have a
DataStructureStrategy abstract parent class,with concrete implementations in subclasses,
for example
DynamicMapStrategy and ArrayMapStrategy.And then, the code to initialize the data structure would look like this:
ds_ = dsStrategy->newDataStructure();where
newDataStructure would be implemented in the specialized subclasses like this:// implementation of DynamicMapStrategy::newDataStructure
return new DynamicMap;
// implementation of ArrayMapStrategy::newDataStructure
return new ArrayMap;In other words:
-
Your hashmap class shouldn't have to know how a strategy is implemented. At no point should the code have conditional statements on
DSType. In fact there's no need for DSType. -
The same goes for all the other kind of strategies that you implied: hash strategy, collision strategy. Define a common interface for each strategy that the hashmap wants to work with. Move the implementations of strategies outside of the hashmap, into subclasses of the common interfaces.
-
These strategies should not change during the lifetime of a hashmap. It wouldn't be efficient to switch the strategy of an existing hashmap. So it would be better to make these strategies required at construction time, and remove the setters.
Code Snippets
template <class K, class V>
void Map<K,V>::SetDataStructureStrategy(int ds_type)
{
if(ds_ != NULL)
{
delete ds_;
}
if(ds_type == DSType::DYNAMIC_MAP)
{
ds_ = new DynamicMap<K,V>;
}
else if(ds_type == DSType::ARRAY_MAP)
{
ds_ = new ArrayMap<K,V>;
}
}template <class K, class V>
void Map<K,V>::SetDataStructureStrategy(DataStructureStrategy * dsStrategy)
{
this->dsStrategy = dsStrategy;
}ds_ = dsStrategy->newDataStructure();// implementation of DynamicMapStrategy::newDataStructure
return new DynamicMap<K,V>;
// implementation of ArrayMapStrategy::newDataStructure
return new ArrayMap<K,V>;Context
StackExchange Code Review Q#71042, answer score: 12
Revisions (0)
No revisions yet.