Anton Korobeynikov wrote:
> +template<class Payload>
> +class Trie {
> + class Edge;
> + class Node;
> +
> + class Edge {
> + std::string Label;
> + Node *Parent, *Child;
The Parent field seems to be unused internally by the Trie
implementation. Assuming the Edge interface is invisible for Trie
clients, may this field be removed?
> + std::vector<Node*> Nodes;
I guess this vector is purely for destruction purposes... Have you
considered storing a root node only and performing destruction by
postorder trie traversal?
> + inline Node* getRoot() const { return Nodes[0]; }
I'm not familiar what Trie structure will be used for, but shouldn't
this method be private? Currently, it exposes a way to call most of the
Node's (and indirectly Edge's) methods, which I suppose, should be
inaccessible to clients.
> + bool addString(const std::string& s, const Payload& data) {
> + Node* cNode = getRoot();
> + Edge* nEdge = NULL;
> + std::string s1(s);
> +
> + while (nEdge == NULL) {
> + if (cNode->Edges.count(s1[0])) {
> + Edge& cEdge = cNode->Edges[s1[0]];
> + typename Edge::QueryResult r = cEdge.query(s1);
> +
> + switch (r) {
> + case Edge::Same:
Do we really want to hit the assert here, and not only return false?
> + case Edge::StringIsPrefix:
Shouldn't the edge be splitted in this case and the payload be attached
to the new node?
-Wojtek
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
