Return to Answer

• node_t: Names ending with the _t suffix are actually reserved by the POSIX standard. See this (right at the end). You should follow the fairly common PascalCase convention for type names. Node would be better.

• Public virtual destructor: Are you extending Octree somewhere? Only provide a public virtual destructor if you intend to do so. If you are not sure, I suggest following the YAGNI principle and making it non-virtual. Programmers are usually pretty bad at predicting future needs.

• Unnamed parameters on function prototypes: Please name the function parameters in the prototypes. This will make your functions self-documented.

• Public root: Doesn't seem like a good idea to leave the root node of the tree as a public field. Makes it very easy for someone to accidentally break an instance of the class by changing the root pointer.

• Nodes (node_t) should have a constructor: You should provide one or more constructors for your node type so you can more easily declare and initialize instances. E.g.: node_t * node = new node_t(42.1f);.

• You are using 0 to assign a null pointer. C++11 now has nullptr. If you have access to it, use nullptr instead. It avoids questionable implicit conversions and is more idiomatic than a literal zero.

• Logic inside loops can be simplified: When checking a condition inside a loop, you can make it simpler and more compact by using a continue or break to avoid an if-else chain. Example:

• node_t: Names ending with the _t suffix are actually reserved by the POSIX standard. See this (right at the end). You should follow the fairly common PascalCase convention for type names. Node would be better.

• Public virtual destructor: Are you extending Octree somewhere? Only provide a public virtual destructor if you intend to do so. If you are not sure, I suggest following the YAGNI principle and making it non-virtual. Programmers are usually pretty bad at predicting future needs.

• Unnamed parameters on function prototypes: Please name the function parameters in the prototypes. This will make your functions self-documented.

• Public root: Doesn't seem like a good idea to leave the root node of the tree as a public field. Makes it very easy for someone to accidentally break an instance of the class by changing the root pointer.

• Nodes (node_t) should have a constructor: You should provide one or more constructors for your node type so you can more easily declare and initialize instances. E.g.: node_t * node = new node_t(42.1f);.

• You are using 0 to assign a null pointer. C++11 now has nullptr. If you have access to it, use nullptr instead. It avoids questionable implicit conversions and is more idiomatic than a literal zero.

• Logic inside loops can be simplified: When checking a condition inside a loop, you can make it simpler and more compact by using a continue or break to avoid an if-else chain. Example:

• node_t: Names ending with the _t suffix are actually reserved by the POSIX standard. See this (right at the end). You should follow the fairly common PascalCase convention for type names. Node would be better.

• Public virtual destructor: Are you extending Octree somewhere? Only provide a public virtual destructor if you intend to do so. If you are not sure, I suggest following the YAGNI principle and making it non-virtual. Programmers are usually pretty bad at predicting future needs.

• Unnamed parameters on function prototypes: Please name the function parameters in the prototypes. This is a form of code self documentation.

• Public root: Doesn't seem like a good idea to leave the root node of the tree as a public field. Makes it very easy for someone to accidentally break an instance of the class by changing the root pointer.

• Nodes (node_t) should have a constructor: You should provide one or more constructors for your node type so you can more easily declare and initialize instances. E.g.: node_t * node = new node_t(42.1f);.

• You are using 0 to assign a null pointer. C++11 now has nullptr. If you have access to it, you should use nullptr instead.

• Logic inside loops can be simplified: When checking a condition inside a loop, you can make it simpler and more compact by using a continue or break to avoid an if-else chain. Example:

   for(int i = 7; i >= 0; i--)
   {
   const int index = ...
   if(!node->child[index])
   {
   node->child[index] = new node_t;
   node = node->child[index];
   node->value = value;
   for(int j = 0; j < 8; j++)
   node->child[j] = 0;
   }
   else
   {
   node = node->child[index];
   }
   }
  

  Could be rewritten as:

   for(int i = 7; i >= 0; i--)
   {
   const int index = ...
   if(node->child[index])
   {
   node = node->child[index];
   continue;
   }
   node->child[index] = new node_t;
   node = node->child[index];
   node->value = value;
   for(int j = 0; j < 8; j++)
   node->child[j] = 0;
   }
  

• node_t: Names ending with the _t suffix are actually reserved by the POSIX standard. See this (right at the end). You should follow the fairly common PascalCase convention for type names. Node would be better.

• Public virtual destructor: Are you extending Octree somewhere? Only provide a public virtual destructor if you intend to do so. If you are not sure, I suggest following the YAGNI principle and making it non-virtual. Programmers are usually pretty bad at predicting future needs.

• Unnamed parameters on function prototypes: Please name the function parameters in the prototypes. This will make your functions self-documented.

• Public root: Doesn't seem like a good idea to leave the root node of the tree as a public field. Makes it very easy for someone to accidentally break an instance of the class by changing the root pointer.

• Nodes (node_t) should have a constructor: You should provide one or more constructors for your node type so you can more easily declare and initialize instances. E.g.: node_t * node = new node_t(42.1f);.

• You are using 0 to assign a null pointer. C++11 now has nullptr. If you have access to it, use nullptr instead. It avoids questionable implicit conversions and is more idiomatic than a literal zero.

• Logic inside loops can be simplified: When checking a condition inside a loop, you can make it simpler and more compact by using a continue or break to avoid an if-else chain. Example:

 for(int i = 7; i >= 0; i--)
 {
 const int index = ...
 
 if(!node->child[index])
 {
 node->child[index] = new node_t;
 node = node->child[index];
 node->value = value;
 for(int j = 0; j < 8; j++)
 node->child[j] = 0;
 }
 else
 {
 node = node->child[index];
 }
 }

Could be rewritten as:
 for(int i = 7; i >= 0; i--)
 {
 const int index = ...
 if(node->child[index])
 {
 node = node->child[index];
 continue;
 }
 
 node->child[index] = new node_t;
 node = node->child[index];
 node->value = value;
 for(int j = 0; j < 8; j++)
 node->child[j] = 0;
 }