Brain fog (#1306)

Topics/tags: Teaching

Today, we take a break from Sam whining about his personal life to hear (read) Sam whine about his professional life.

One of the many commitments I made this semester was to provide my CSC-207 students with reasonable sets of unit tests for each homework assignment (mini-project, in course parlance). I’d rather students know about potential issues in their assignments before they turn them in for grading rather than after they have them graded. That way, they can fix them before turning them in, and everyone’s job is easier.

Unfortunately, writing unit tests remains a non-trivial task. At larger institutions, faculty probably dump the task on their teaching assistants. My mentors don’t really have the time to do so. I must admit that I also don’t necessarily trust others to write tests. I am, after all, a control freak.

I had already spent more than a day writing unit tests for our latest mini-project. Today, I was working on additional tests for the eqv method. Students are building structures (somewhat like graphical UI structures, but with ASCII). We are considering three types of equality of such structures: two things look the same (equal), two things were built in essentially the same way (eqv), and “two things occupy the same memory location (eq). I’ve provided students with equal and eq, and I’ve asked them to write eqv, possibly the hardest of the three.

To test their code, I’m making a lot of objects that all look the same but are constructed differently. Sometimes, that construction is very similar. Sometimes it’s very different.

Here’s a method I’d written that compares grids that are built in different ways. Warning! Horrid code follows!

  /**
   * Grids vs grids that are different.
   */
  public void exesGridVsGridDiff() {
    assertTrue(AsciiBlock.equal(exesGrid0a, exesGrid1a),
        "*: equal(exesGrid0a, exesGrid1a)");
    assertTrue(AsciiBlock.equal(exesGrid1a, exesGrid0a),
        "*: equal(exesGrid1a, exesGrid0a)");
    assertTrue(AsciiBlock.equal(exesGrid0a, exesGrid2a),
        "*: equal(exesGrid0a, exesGrid2a)");
    assertTrue(AsciiBlock.equal(exesGrid2a, exesGrid0a),
        "*: equal(exesGrid2a, exesGrid0a)");
    assertTrue(AsciiBlock.equal(exesGrid1a, exesGrid2a),
        "*: equal(exesGrid0a, exesGrid2a)");
    assertTrue(AsciiBlock.equal(exesGrid2a, exesGrid1a),
        "*: equal(exesGrid2a, exesGrid0a)");
    assertTrue(AsciiBlock.equal(exesGrid0a, exesGrid3a),
        "*: equal(exesGrid0a, exesGrid3a)");
    assertTrue(AsciiBlock.equal(exesGrid3a, exesGrid0a),
        "*: equal(exesGrid3a, exesGrid0a)");
    assertTrue(AsciiBlock.equal(exesGrid1a, exesGrid3a),
        "*: equal(exesGrid1a, exesGrid3a)");
    assertTrue(AsciiBlock.equal(exesGrid3a, exesGrid1a),
        "*: equal(exesGrid3a, exesGrid1a)");
    assertTrue(AsciiBlock.equal(exesGrid2a, exesGrid3a),
        "*: equal(exesGrid2a, exesGrid3a)");
    assertTrue(AsciiBlock.equal(exesGrid3a, exesGrid2a),
        "*: equal(exesGrid3a, exesGrid2a)");

    assertFalse(AsciiBlock.eqv(exesGrid0a, exesGrid1a),
        "M: eqv(exesGrid0a, exesGrid1a)");
    assertFalse(AsciiBlock.eqv(exesGrid1a, exesGrid0a),
        "M: eqv(exesGrid1a, exesGrid0a)");
    assertFalse(AsciiBlock.eqv(exesGrid0a, exesGrid2a),
        "M: eqv(exesGrid0a, exesGrid2a)");
    assertFalse(AsciiBlock.eqv(exesGrid2a, exesGrid0a),
        "M: eqv(exesGrid2a, exesGrid0a)");
    assertFalse(AsciiBlock.eqv(exesGrid1a, exesGrid2a),
        "M: eqv(exesGrid0a, exesGrid2a)");
    assertFalse(AsciiBlock.eqv(exesGrid2a, exesGrid1a),
        "M: eqv(exesGrid2a, exesGrid0a)");
    assertFalse(AsciiBlock.eqv(exesGrid0a, exesGrid3a),
        "M: eqv(exesGrid0a, exesGrid3a)");
    assertFalse(AsciiBlock.eqv(exesGrid3a, exesGrid0a),
        "M: eqv(exesGrid3a, exesGrid0a)");
    assertFalse(AsciiBlock.eqv(exesGrid1a, exesGrid3a),
        "M: eqv(exesGrid1a, exesGrid3a)");
    assertFalse(AsciiBlock.eqv(exesGrid3a, exesGrid1a),
        "M: eqv(exesGrid3a, exesGrid1a)");
    assertFalse(AsciiBlock.eqv(exesGrid2a, exesGrid3a),
        "M: eqv(exesGrid2a, exesGrid3a)");
    assertFalse(AsciiBlock.eqv(exesGrid3a, exesGrid2a),
        "M: eqv(exesGrid3a, exesGrid2a)");
  } // exesGridvsGridDiff

Isn’t that long and repetitious?

For something different yet similar, let’s look at the code for comparing rectangles to horizontal compositions. Once again, every shape I make looks the same but is constructed differently. There’s only one kind of rectangle (exesRect0a) and seven kinds of horizontal compositions (exesHComp0a through exesHComp6a), at least right now. Warning! More horrid code follows.

  /**
   * Rectangles vs HComps.
   */
  @Test
  public void exesRectVsHComp() {
    assertTrue(AsciiBlock.equal(exesRect0a, exesHComp0a),
        "*: equal(exesRect0a, exesHComp0a)");
    assertTrue(AsciiBlock.equal(exesRect0a, exesHComp1a),
        "*: equal(exesRect0a, exesHComp1a)");
    assertTrue(AsciiBlock.equal(exesRect0a, exesHComp2a),
        "*: equal(exesRect0a, exesHComp2a)");
    assertTrue(AsciiBlock.equal(exesRect0a, exesHComp3a),
        "*: equal(exesRect0a, exesHComp3a)");
    assertTrue(AsciiBlock.equal(exesRect0a, exesHComp4a),
        "*: equal(exesRect0a, exesHComp4a)");
    assertTrue(AsciiBlock.equal(exesRect0a, exesHComp5a),
        "*: equal(exesRect0a, exesHComp5a)");
    assertTrue(AsciiBlock.equal(exesRect0a, exesHComp6a),
        "*: equal(exesRect0a, exesHComp6a)");

    assertFalse(AsciiBlock.eqv(exesRect0a, exesHComp0a),
        "M: eqv(exesRect0a, exesHComp0a)");
    assertFalse(AsciiBlock.eqv(exesRect0a, exesHComp1a),
        "M: eqv(exesRect0a, exesHComp1a)");
    assertFalse(AsciiBlock.eqv(exesRect0a, exesHComp2a),
        "M: eqv(exesRect0a, exesHComp2a)");
    assertFalse(AsciiBlock.eqv(exesRect0a, exesHComp3a),
        "M: eqv(exesRect0a, exesHComp3a)");
    assertFalse(AsciiBlock.eqv(exesRect0a, exesHComp4a),
        "M: eqv(exesRect0a, exesHComp4a)");
    assertFalse(AsciiBlock.eqv(exesRect0a, exesHComp5a),
        "M: eqv(exesRect0a, exesHComp5a)");
    assertFalse(AsciiBlock.eqv(exesRect0a, exesHComp6a),
        "M: eqv(exesRect0a, exesHComp6a)");
  } // exesRectVsHComp()

I was about to start writing the incredibly similar compare grids to horizontal compositions—which involves four kinds of grids and seven kinds of horizontal compositions—and realized that I was, once again, copying, pasting, and changing large chunks of code. And, as I tell my students, If you are programming with copy, paste, and change, you should find something better.

I’m an experienced enough programmer that I should have started with something better. However, it seems I’m experiencing some brain fog. So I began by writing bad code. Fortunately, I eventually realized I needed to clean it up, and I did so before I wrote too much bad code.

What are the fixes? First, when dealing with many related values, you should put them in an array. I put all the exesGrid#a objects (e.g., exesGrid2a) in one array (exesGridsA) and all the exesGrid#b objects in another (exesGridsB). I did something similar for the other types.

Second, you should then write for loops to deal with the pairs of objects. If possible, you should also put those loops in separate methods. (Looping and functions/methods are two of the most common ways to avoid repetitive code.) Here are the two methods I wrote. Hopefully, I got them right. The first compares different objects of the same type.

  /**
   * Compare different blocks in the same type for equivalence.
   * The assumption is that all pairs should be different.
   *
   * @param blocks
   *   The collection of blocks to compare.
   *
   * @param type
   *   The type of elements (e.g., "Rect" or "HComp"). Used for messages.
   */
  void exesCompareDifferent(AsciiBlock[] blocks, String type) {
    for (int i = 0; i < blocks.length; i++) {
      for (int j = i + 1; j < blocks.length; j++) {
        assertTrue(AsciiBlock.equal(blocks[i], blocks[j]),
          String.format("*: equal(exes%s%da, exes%s%da)",
              type, i, type, j));
        assertTrue(AsciiBlock.equal(blocks[j], blocks[i]),
          String.format("*: equal(exes%s%da, exes%s%da)",
              type, j, type, i));
        assertFalse(AsciiBlock.eqv(blocks[i], blocks[j]),
          String.format("M: eqv(exes%s%da, exes%s%da)",
              type, i, type, j));
        assertFalse(AsciiBlock.eqv(blocks[j], blocks[i]),
          String.format("M: eqv(exes%s%da, exes%s%da)",
              type, j, type, i));
      } // for j
    } // for i
  } // exesCompareDifferent(AsciiBlock[], String)

Once I’d written that, the test for grids vs. grids became remarkably small.

  /**
   * Grids vs grids that are different.
   */
  @Test
  public void exesGridVsGridDiff() {
    exesCompareDifferent(exesGridsA, "Grid");
  } // exesGridvsGridDiff

The second bit of general code compares objects of different types.

  /**
   * Compare different blocks in the different type for equivalence.
   * The assumption is that all pairs should be different.
   *
   * @param blocksOne
   *   The first collection of blocks to compare.
   * @param typeOne
   *   The type of elements in blocksOne (e.g., "Rect" or "HComp").
   *   Used for messages.
   * @param blocksTwo
   *   The second collection of blocks to compare.
   * @param typeOne
   *   The type of elements in blocksTwo (e.g., "Rect" or "HComp").
   *   Used for messages.
   */
  void exesCompareDifferent(AsciiBlock[] blocksOne, String typeOne,
      AsciiBlock[] blocksTwo, String typeTwo) {
    for (int i = 0; i < blocksOne.length; i++) {
      for (int j = 0; j < blocksTwo.length; j++) {
        assertTrue(AsciiBlock.equal(blocksOne[i], blocksTwo[j]),
          String.format("*: equal(exes%s%da, exes%s%da)",
              typeOne, i, typeTwo, j));
        assertTrue(AsciiBlock.equal(blocksTwo[j], blocksOne[i]),
          String.format("*: equal(exes%s%da, exes%s%da)",
              typeTwo, j, typeOne, i));
        assertFalse(AsciiBlock.eqv(blocksOne[i], blocksTwo[j]),
          String.format("M: eqv(exes%s%da, exes%s%da)",
              typeOne, i, typeTwo, j));
        assertFalse(AsciiBlock.eqv(blocksTwo[j], blocksOne[i]),
          String.format("M: eqv(exes%s%da, exes%s%da)",
              typeTwo, j, typeOne, i));
      } // for j
    } // for i
  } // exesCompareDifferent(AsciiBlock[], String)

And that too-long code for rectangles vs. horizontal compositions becomes something much shorter.

  /**
   * Rectangles vs HComps.
   */
  @Test
  public void exesRectVsHComp() {
    exesCompareDifferent(exesRectsA, "Rect", exesHCompsA, "HComp");
  } // exesRectVsHComp()

I could even write a loop of calls to exesCompareDifferent, using an array of arrays to loop through. But that seems like a bit of overkill. I like seeing separate tests for different pairings.

There are also other benefits to this approach. Most importantly, if I decided to add an eighth kind of horizontal composition, I would only need to make one or two changes in the program: adding an element to exesHCompsA (as well as an exesHCompsB that I haven’t mentioned yet). No need to search my code for other lines to add! (Let’s see … everywhere I mention exesHComp6a, I should add a similar assertion for exesHZComp7a ….) Making additional examples easy to add encourages more testing (or at least doesn’t discourage more testing), and more testing is generally good (within limits). In fact, while testing the tests, as it were, by writing nearly correct code, I discovered a few more cases to add.

In addition, if we suddenly added a new type to our project, it wouldn’t take me long to add tests for that type. Without the general methods, I’d be spending time repeatedly copying and pasting and changing.

The new approach also cuts the number of lines in the program. That makes the program more readable. It also reminds us that lines per hour is a bad way to measure programmer productivity; sometimes the best code (or even just decent code) cuts the number of lines, rather than increasing them.

There is, however, a significant difficulty in implementing this approach. When we’ve written separate assertions for each pairing it’s easy to tell which pair of values failed the test. If we weren’t careful to generate good messages, we’d just see a report that we failed on one of the pairs, but be unable to tell which one. That would be sad. As you may be able to tell, I spent a bit of extra effort ensuring that the error message in the assertion makes it clear which pair we have to consider. That approach makes it necessary that I follow some (undocumented) naming conventions that I was already following.

I think that’s it for now. Do I still have brain fog? Almost certainly. In fact, I just typed brain bog. But I’m glad that I still have enough ability to peer through the fog that I can remember how to fix things. I just wish that fog didn’t look like the composition of f and g.

Version 1.0 of 2024-09-25.