Search code examples
c++ooptreeencapsulationavl-tree

Is it possible combining symmetric code pieces?


I was implementing avl trees for my school projects and found myself writing almost same code twice for symmetric situations. For example, this function performs rotation of two nodes to balance the tree. If clause handles the case in which lower node is a left child of higher one, and the else clause handles the opposite:

void avl<T>::rotate(node<T> *x, node<T> *y)
{
  if (x == y->l)
  {
    x->p = y->p;
    if (y->p != nullptr)
      if(y->p->l == y)
        y->p->l = x;
      else
        y->p->r = x;
    else
      this->setHead(x);
    y->p = x;
    y->l = x->r;
    if(x->r != nullptr)
      x->r->p = y;
    x->r = y;
    y->dl = y->calcd('l');
    x->dr = x->calcd('r');
    if(x->p != nullptr)
      if(x->p->l == x)
        x->p->dl = x->p->calcd('l');
      else
        x->p->dr = x->p->calcd('r');
  }
  else
  {
    x->p = y->p;
    if (y->p != nullptr)
      if(y->p->r == y)
        y->p->r = x;
      else
        y->p->l = x;
    else
      this->setHead(x);
    y->p = x;
    y->r = x->l;
    if(x->l != nullptr)
      x->l->p = y;
    x->l = y;
    y->dl = y->calcd('l');
    x->dr = x->calcd('r');
    if(x->p != nullptr)
      if(x->p->r == x)
        x->p->dr = x->p->calcd('r');
      else
        x->p->dl = x->p->calcd('l');

  }
}

As you can see, the else clause is exactly similar to the if clause with 'l' and 'r' swapped. Is there a way to combine them. What can I do to improve on it? Is there some design mistake in my code?


Solution

  • Use pointers to members. The only difference between the two branches is which member you're accessing, so that's the easy way to abstract that out:

    using Child = node<T> node<T>::*;
    
    void rotate_impl(node<T>* x, node<T>* y, Child* left, Child* right)
    {
        x->p = y->p;
        if (y->p != nullptr) {
          if(y->p->*left == y) {
            y->p->*left = x;
          }
          else {
            y->p->*right = x;
          }
        }
        else {
          this->setHead(x);
        }
    
        y->p = x;
        y->*left = x->*right;
    
        if(x->*right != nullptr) {
          (x->*right)->p = y;
        }
    
        x->*right = y;
        y->dl = y->calcd('l');
        x->dr = x->calcd('r');
        if(x->p != nullptr) {
          if(x->p->*left == x) {
            x->p->dl = x->p->calcd('l');
          }
          else {
            x->p->dr = x->p->calcd('r');
          }
       }
    }
    
    void avl<T>::rotate(node<T> *x, node<T> *y)
    {
      if (x == y->l) {
        rotate_impl(x, y, &node<T>::l, &node<T>::r);
      }
      else {
        rotate_impl(x, y, &node<T>::r, &node<T>::l);
      }
    }
    

    I also took the liberty of adding braces to your code. You can thank me later.