Refactoring Demo v17

From J. Arrizza Wiki

Jump to: navigation, search

Having trouble figuring out what the snippet does. Added a couple of test cases.

TEST(deletefromleft)
  {
  Comparer::StringList leftlist;
  Comparer::StringList rightlist;

  leftlist.clear();
  rightlist.clear();

  rightlist.push_back("item1");
  rightlist.push_back("item4");
  leftlist.push_back("item1");
  leftlist.push_back("item2");
  leftlist.push_back("item3");
  leftlist.push_back("item4");
  
  int result;
  Comparer cmp;
  cmp.Init(leftlist, rightlist);

  result = cmp.compare();
  utassert(Comparer::DEL, result);
  utassert(2, cmp.leftDiff.size());
  utassert(0, cmp.rightDiff.size());
  utassert("item2", cmp.leftDiff[0]);
  utassert("item3", cmp.leftDiff[1]);

  result = cmp.compare();
  utassert(cmp.DONE, result);
  }

TEST(insertintoleft)
  {
  Comparer::StringList leftlist;
  Comparer::StringList rightlist;

  leftlist.clear();
  rightlist.clear();

  rightlist.push_back("item1");
  rightlist.push_back("item2");
  rightlist.push_back("item3");
  rightlist.push_back("item4");
  leftlist.push_back("item1");
  leftlist.push_back("item4");
  
  int result;
  Comparer cmp;
  cmp.Init(leftlist, rightlist);

  result = cmp.compare();
  utassert(Comparer::INS, result);
  utassert(0, cmp.leftDiff.size());
  utassert(2, cmp.rightDiff.size());
  utassert("item2", cmp.rightDiff[0]);
  utassert("item3", cmp.rightDiff[1]);

  result = cmp.compare();
  utassert(cmp.DONE, result);
  }

Took another look at:

  bool done = false;
  for(int tmp = mLeftIndex + 1; tmp < mLeftList.size(); tmp++)
    {
    if (mLeftList[tmp] != mRightList[mRightIndex])
      continue;
    
    while (mLeftIndex != tmp)
      leftDiff.push_back(mLeftList[mLeftIndex++]);
    done = true;
    result = DEL;
    }

The inner while() is invoked only if the current item from the right list is found somewhere further on in the left list. Then all of the left list items are pushed onto the diff list until we reach the matching item. But then we keep searching! This looks like a bug. We should be exiting the loop, I think.

I need to write a test case to verify it is a bug. The lists need to be something like this:

    left     right
    item1    item1
    item2    
    item3    item3 
    item4    
    item3
    item5    item5

If there is no bug, it should return two deleted sections: one with item2 in it and a second with item4 in it. If there is a bug, it will return one section with two items in it: item2 and item4.

Personal tools