HiveBrain v1.2.0
Get Started
← Back to all entries
principlecppModerate

Strategy pattern - design hash table

Submitted by: @import:stackexchange-codereview··
0
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

Solution

You're misusing the term strategy pattern.
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.