April, 2012

  • The Old New Thing

    Introducing the unrolled-switch anti-pattern

    • 39 Comments

    Over the years, I've seen a bunch of coding anti-patterns. I figured maybe I'll share a few.

    Today, I'll introduce what I'm calling the unrolled-switch anti-pattern, also known as "Specialization is always faster, right?"

    enum Axis
    {
        XAxis,
        YAxis,
        ZAxis,
    };
    
    // code earlier in the function ensure that
    // "axis" is always a valid axis
    int newPosition;
    switch (axis)
    {
    case XAxis:
        newPosition = m_position[XAxis] + amount;
        if (newPosition < m_minPosition[XAxis])
            newPosition = m_minPosition[XAxis];
        if (newPosition > m_maxPosition[XAxis])
            newPosition = m_maxPosition[XAxis];
        m_position[XAxis] = amount;
        break;
    case YAxis:
        newPosition = m_position[YAxis] + amount;
        if (newPosition < m_minPosition[YAxis])
            newPosition = m_minPosition[YAxis];
        if (newPosition > m_maxPosition[YAxis])
            newPosition = m_maxPosition[YAxis];
        m_position[YAxis] = amount;
        break;
    case ZAxis:
        newPosition = m_position[ZAxis] + amount;
        if (newPosition < m_minPosition[ZAxis])
            newPosition = m_minPosition[ZAxis];
        if (newPosition > m_maxPosition[XAxis])
            newPosition = m_maxPosition[XAxis];
        m_position[ZAxis] = amount;
        break;
    }
    
    As we all know, special-case code is faster than general-purpose code. Instead of writing slow general-purpose code:

        newPosition = m_position[axis] + amount;
        if (newPosition < m_minPosition[axis])
            newPosition = m_minPosition[axis];
        if (newPosition > m_maxPosition[axis])
            newPosition = m_maxPosition[axis];
        m_position[axis] = amount;
    

    we unroll it into a switch statement, thereby generating highly-optimized special-purpose code, one for each axis.

    What makes this anti-pattern particularly frustrating is that you cannot tell at a glance whether all the cases really are the same (just with different axes).

    In fact, they aren't.

    If you look closely, you'll see that we check the new Z-position against the X-axis maximum rather than the Z-axis maximum. If you're reading this code, you now start to wonder, "Is this a copy/paste bug, or is there some reason that we really do want to check the Z-position against the X-axis minimum?"

    A variation on the unrolled-switch is the unrolled-if, used if the item you want to unroll cannot be used in a switch statement:

    FruitBasket *BananaBasket;
    FruitBasket *AppleBasket;
    FruitBasket *PearBasket;
    FruitBasket *MangoBasket;
    
    if (basket == BananaBasket) {
      if (!BananaBasket->IsEmpty()) {
        fruit = BananaBasket->TakeFruit();
        if (HaveKnife()) {
          TakeKnife();
          fruit->Peel();
          fruit->Slice();
          fruit->Eat();
          ReplaceKnife();
        } else {
          BananaBasket->AddFruit(fruit);
        }
      }
    } else if (basket == AppleBasket) {
      if (!AppleBasket->IsEmpty()) {
        fruit = AppleBasket->TakeFruit();
        if (HaveKnife()) {
          TakeKnife();
          fruit->Peel();
          fruit->Slice();
          fruit->Eat();
          ReplaceKnife();
        } else {
          AppleBasket->AddFruit(fruit);
        }
      }
    } else if (basket == PearBasket) {
      if (!PearBasket->IsEmpty()) {
        fruit = PearBasket->TakeFruit();
        if (HaveKnife()) {
          TakeKnife();
          fruit->Slice();
          fruit->Eat();
          ReplaceKnife();
        } else {
          PearBasket->AddFruit(fruit);
        }
      }
    } else if (basket == MangoBasket) {
      if (!MangoBasket->IsEmpty()) {
        fruit = MangoBasket->TakeFruit();
        if (HaveKnife()) {
          TakeKnife();
          fruit->Peel();
          fruit->Slice();
          fruit->Eat();
          ReplaceKnife();
        } else {
          BananaBasket->AddFruit(fruit);
        }
      }
    }
    

    When I pointed out in an aside to the customer that this could be simplified (after fixing the copy/paste errors) to

    if (!basket->IsEmpty()) {
      fruit = basket->TakeFruit();
      if (HaveKnife()) {
        TakeKnife();
        fruit->Peel();
        fruit->Slice();
        fruit->Eat();
        ReplaceKnife();
      } else {
        basket->AddFruit(fruit);
      }
    }
    

    the response was, "Hey, that's a neat trick. I didn't realize you could do that."

    I wonder if this person also programs loops like this:

    switch (limit)
    {
    case 0:
      break;
    case 1:
      do_something(array[0]);
      break;
    case 2:
      for (int i = 0; i < 2; i++) do_something(array[i]);
      break;
    case 3:
      for (int i = 0; i < 3; i++) do_something(array[i]);
      break;
    case 4:
      for (int i = 0; i < 4; i++) do_something(array[i]);
      break;
    case 5:
      for (int i = 0; i < 5; i++) do_something(array[i]);
      break;
    case 6:
      for (int i = 0; i < 6; i++) do_something(array[i]);
      break;
    ...
    case 999:
      for (int i = 0; i < 999; i++) do_something(array[i]);
      break;
    default:
      FatalError("Need more cases to handle larger array");
      break;
    }
    
  • The Old New Thing

    If posting here is frequently frustrating and irritating, why do I keep doing it?

    • 50 Comments

    Appreciator wonders, if I find posting here frequently frustrating and irritating, why I keep doing it anyway?

    Imagine I announced one day, "This is too frustrating and annoying. I'm going to stop now." To the rest of the world, this would "mean something." People would discuss in hushed tones—and for the Internet, hushed tones means in a normal voice, or perhaps even louder than normal—what this "means" for blogging, for Microsoft, for whatever. People would start speculating as to what pushed me over the line, maybe muse about what this means for other bloggers, or question my actual motivations. "Is this really a cover so Raymond can quit Microsoft and work for another company?" It's easier just to avoid becoming news by not doing anything newsworthy.

    I guess I could stop if I made up some bogus but less controversial reason for stopping, say, because I wanted to "spend more time with my family."

    Generally speaking, change is news. Contrapositively, no-news requires no-change. I prefer not to be news.

    (In the same way that if I decided to change my policy and start opinionating more, people would make more of the change than if I had been opinionated from the beginning. I often envy Michael Kaplan for having established himself as an opinionated blowhard early on, which gives him the freedom to spout off on whatever he wants without creating much controversy.)

Page 3 of 3 (22 items) 123