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

C++ Trie using std::map and std::unique_ptr

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
stdmapusingtrieandunique_ptr

Problem

I'm learning C++ and whilst trying to write in modern C++, I've made an attempt to rewrite a trie implementation written in C found here: http://www.geeksforgeeks.org/trie-insert-and-search/

It uses arrays to hold the branches in each node and because it's C it was done using malloc() and no freeing of memory was done in the example.

Is this an efficient approach of representing a trie in C++11? what other ways can I store children in trie nodes?

```
#include
#include
#include
#include

class Trie {
struct Node;
typedef std::unique_ptr spNode;
struct Node {
std::map children;
bool isLeaf;
Node() : isLeaf{false} {}
};

spNode root;
public:
Trie();
void insert(const std::string& str);
bool search(const std::string& str);
};

Trie::Trie():root{nullptr}{}

void Trie::insert(const std::string& str) {
if (root == nullptr) {
std::unique_ptr node(new Node());
root = std::move(node);
}

Node *temp = root.get();
for (const char& c : str) {
if (temp->children.find(c) == temp->children.end()) {//if char not in map
std::unique_ptr node(new Node());
temp->children[c] = std::move(node);
}

temp = temp->children[c].get();
}

temp->isLeaf = true;
}

bool Trie::search(const std::string &str) {
if (root == nullptr) return false;

Node *temp = root.get();
for (const char& c : str) {
if (temp->children.find(c) == temp->children.end())
return false;
temp = temp->children[c].get();
}
return (temp->isLeaf);
}

int main (void) {
std::string words[] = { "Hello", "hi", "hey", "howdy", "ho"};
Trie test;

for (const auto& str : words) {
test.insert(str);
}

if (test.search("Hello"))
std::cout << " 'Hello' is found in the trie\n";
else
std::cout <<" 'Hello' is not found in the trie\n";

if (test.search("yo"))
std::cout << " 'yo' is found in the t

Solution

I don't see the need for unique_ptr for the root node.

spNode root;


I would just make this a node.

Node root;


The use of std::map is fine. But it does have O(log(n)) lookup. If you switch back to an array its O(1). Its a time for space thing. Pick the one you want.

I don't like the two line creation of nodes.

std::unique_ptr node(new Node());
        temp->children[c] = std::move(node);


Just use reset:

temp->children[c].reset(new Node());


Or if you have C++14 use std::make_unique()

temp->children[c] = std::make_unique();


Personally search() does not seem quite the correct verb.

bool search(const std::string& str);


What is wrong with find()?

Code Snippets

spNode root;
std::unique_ptr<Node> node(new Node());
        temp->children[c] = std::move(node);
temp->children[c].reset(new Node());
temp->children[c] = std::make_unique<Node>();
bool search(const std::string& str);

Context

StackExchange Code Review Q#163226, answer score: 7

Revisions (0)

No revisions yet.