Refactoring Experiment Continues …

I’m sitting on the balcony at Folkvang Castle in Lexicolo. NightShade just got finished testing her new sword, and Dizzi is working on tuning the route cards for one of our vehicles. I thought I’d continue with refactoring the script for the horse’s legs.

Working incrementally …

I’d like to emphasize a couple of things right now. First of all, I haven’t really set a specific plan for improving this code. My practice doesn’t require me to do that. Instead, I look at the code as it is, see things that could be improved, and find safe ways to make those changes. Over time, the code gets more and more like I would like it to look. I call that “better”, but you may not.

That’s my second point. Sometimes, I’ll make a change to the code that seems likely to make it better–by my standards–and it doesn’t look better. No problem, I just change it back and perhaps try something else. I’m not committed to some “grand design”, just to making the code be more clear and better organized, as I understand those ideas.

Maybe that seems disorganized to you. You may have been taught to design everything carefully at the beginning, and work toward your design. For me, it’s just not possible to imagine some perfect design and then have the program turn out that way. The program always seems to have its own mind. In addition, as I’ve practiced this approach of just changing the next little thing, I’ve been surprised to find that at the end of doing it for a while, the programs turn out quite nice.

I hope it’ll turn out that way with this one. Even if it doesn’t, I’m just working on the approach, so I’ll claim that was my point. But I would like the program to be “better”. More beautiful by my standards. Pray for me. :)

Where were we?

When we last met, we had written a test and made it work. In the course of writing the test, I needed the ability to create a list of which links had been turned on, and which had been turned off. To make that happen, I changed the step3() function from this:

step3 ()
{
    llSetLinkAlpha(LF2, 0, ALL_SIDES);
    llSetLinkAlpha(LF3, 1, ALL_SIDES);

    llSetLinkAlpha(RL2, 0, ALL_SIDES);
    llSetLinkAlpha(RL3, 1, ALL_SIDES); 

    llSetLinkAlpha(RF2, 0, ALL_SIDES);
    llSetLinkAlpha(RF3, 1, ALL_SIDES);

    llSetLinkAlpha(RR2, 0, ALL_SIDES);
    llSetLinkAlpha(RR3, 1, ALL_SIDES);
}

to this:

step3 ()
{
    setInvisible(LF2);
    setVisible  (LF3);

    setInvisible(RL2);
    setVisible  (RL3); 

    setInvisible(RF2);
    setVisible  (RF3);

    setInvisible(RR2);
    setVisible  (RR3);
}

Then inside the setInvisible() and setVisible() functions, I did the llSetLinkAlpha() call, and also added the item to a global list, like this:

setVisible(integer link) {
    llSetLinkAlpha(link, 1, ALL_SIDES);
    visible += [link];
}

Now by my standards, the change to the step3() function is already an improvement to the code. It expresses intention better, by saying “set invisible” or “set visible”, plus it reduces duplication in the code by pushing all those extra parameters down into the set functions. I like that, and would have made a similar change even if the test had not made it necessary. I’m left at the moment with this undesirable side effect of building up the lists that are only for testing, but we’ll leave worrying about that until later. In the worst case, I’ll just remove all the test scaffolding from the final version.

What’s Next?

What we have done so far is figure out how to get the program under test. We did that by testing one of the functions. Since I plan to be able to modify the whole program, I need to get the whole program under test. That is done by just testing each of the other step() functions and the stand() function, plus anything else that’s in there that  may not be remembering now. We’ll see as we go.

I’m going to proceed to test all the steps, and will only pause between here and “everything under test” if something interesting comes up. Here goes …

OK, it turns out that I do have a little bit to report along the way. As I’m sitting here on the balcony with Dizzi and Night, the repeated llSay(0,…) calls of my test got irritating. I also decided to clean up those ugly messages. Here’s the new code for the asserEqualst:
para

assertEqual(list a, list b) {
    if ( listsAreEqual( a, b ) ) {
        say("success");
    } else {
        say("Lists are not equal, [" + llList2CSV(a) + "] != ]" + llList2CSV(b) + "]");
    }
}

say(string s) {
    llOwnerSay(s);
}

I used llList2CSV and some added square brackets to improve the format of the message, then wrote a little function say() to speak the message using llOwnerSay() instead of llSay(), to keep down the spam. Whenever I make this particular change, I always write a little function say() like this, so I can switch the messages all back and forth if I want to. This is another tiny example of removing duplication. Now the messages come out like this:

Refactoring for Dizzi: success
Refactoring for Dizzi: success
Refactoring for Dizzi: Lists are not equal, [7, 10, 11, 20] != [4, 15, 17, 19]
Refactoring for Dizzi: Lists are not equal, [9, 14, 12, 21] != [7, 10, 11, 20]

That’s better. I also wrote a new test, as you can see. It was for step4(). The test and code look like this:

testStep4() {
    visible = [];
    invisible = [];
    step4();
    assertEqual(visible, [4, 15, 17, 19]);
    assertEqual(invisible, [7, 10, 11, 20]);
}

step4 ()
{
    setInvisible(LF3);
    setVisible(LF4);

    setInvisible(RL3);
    setVisible(RL4);

    setInvisible(RF3);
    setVisible(RF4);

    setInvisible(RR3);
    setVisible(RR4);
}

That’s pretty obvious, I hope, but I do want to comment on one thing. Notice that in the tests, I’ve put in the explicit numbers of the links, which I got from reading Dizzi’s original constant definitions at the front of the script. It would have been easier to put in the named values, LF3, and so on. But since I’ll probably be doing some of this refactoring by cutting and pasting chunks of code around, one possible error would be that I’d accidentally duplicate the wrong named constants over and the test would work but the code not work. So although it takes me a few moments longer, I think testing the actual values is safer.

I was reminded why I do things that way just now. To create testStep4(), I copied testStep3() and then edited it. I ran it, and saw that it failed, as it should, since I had not converted the step4() function. Then I converted step4() and it still failed! After a while I realized I had not changed the line in the test that said step3() to say step4(). For me at least, this sort of problem often happens when I copy and paste, and I’m glad the tests are there to help me find it.

OK. I’ll move on with the intention to write the rest of the tests and make them work. I’ll be back when I’m done, or when something else interesting happens. My choice on what “interesting” is. Sorry about that.

Step 2 is a bit different …

I notice as I’m doing step2() that it is a bit different, in that it clears both step5()’s work and also clears stand(), because it can be entered either after standing or as part of the 2,3,4,5 cycle. I don’t think it’ll make any difference to my tests, except that I’ll have to test for a couple of invisibles instead of just four.

… that works just fine. I’ll bring you up to date on the code later. Now on to step5() and then stand() … OK, that went smoothly. Take a look at how the program stands now. I’ll call your attention to a couple of things down below.

The program now …

integer Root      = 1;

integer LF2       = 9;
integer LF3       = 7;
integer LF4       = 4;
integer LF5       = 8;
integer LF_STAND  = 4;

integer RL2       = 14;
integer RL3       = 10;
integer RL4       = 15;
integer RL5       = 13;
integer RL_STAND  = 10;

integer RF2       = 12;
integer RF3       = 11;
integer RF4       = 17;
integer RF5       = 16;
integer RF_STAND  = 12;

integer RR2       = 21;
integer RR3       = 20;
integer RR4       = 19;
integer RR5       = 18;
integer RR_STAND  = 18;

integer running;

integer num;

stand ()
{
    setVisible(LF_STAND);
    setVisible(RL_STAND);
    setVisible(RF_STAND);
    setVisible(RR_STAND);

    setInvisible(LF2);
    setInvisible(LF3);
    setInvisible(LF5);

    setInvisible(RL2);
    setInvisible(RL4);
    setInvisible(RL5);

    setInvisible(RF3);
    setInvisible(RF4);
    setInvisible(RF5);

    setInvisible(RR2);
    setInvisible(RR3);
    setInvisible(RR4);
}

step2 ()
{
    setVisible(LF2);
    setInvisible(LF4); //stand
    setInvisible(LF5);

    setVisible(RL2);
    setInvisible(RL3); //stand
    setInvisible(RL5);

    setVisible(RF2); //stand
    setInvisible(RF5);

    setVisible(RR2);
    setInvisible(RR5); //stand
}

step3 ()
{
    setInvisible(LF2);
    setVisible  (LF3);

    setInvisible(RL2);
    setVisible  (RL3); 

    setInvisible(RF2);
    setVisible  (RF3);

    setInvisible(RR2);
    setVisible  (RR3);
}

step4 ()
{
    setInvisible(LF3);
    setVisible(LF4);

    setInvisible(RL3);
    setVisible(RL4);

    setInvisible(RF3);
    setVisible(RF4);

    setInvisible(RR3);
    setVisible(RR4);
}

step5 ()
{
    setInvisible(LF4);
    setVisible(LF5);

    setInvisible(RL4);
    setVisible(RL5);

    setInvisible(RF4);
    setVisible(RF5);

    setInvisible(RR4);
    setVisible(RR5);
}

// JR Testing Starts Here

list visible;
list invisible;

assertEqual(list a, list b) {
    if ( listsAreEqual( a, b ) ) {
        say("success");
    } else {
        say("Lists are not equal, [" + llList2CSV(a) + "] != [" + llList2CSV(b) + "]");
    }
}

say(string s) {
    llOwnerSay(s);
}

setInvisible(integer link) {
    llSetLinkAlpha(link, 0, ALL_SIDES);
    invisible += [link];
}

setVisible(integer link) {
    llSetLinkAlpha(link, 1, ALL_SIDES);
    visible += [link];
}

testStep2() {
    visible = [];
    invisible = [];
    step2();
    assertEqual(visible, [9, 14, 12, 21]);
    assertEqual(invisible, [4, 8, 10, 13, 16, 18]);
}

testStep3() {
    visible = [];
    invisible = [];
    step3();
    assertEqual(visible, [7, 10, 11, 20]);
    assertEqual(invisible, [9, 14, 12, 21]);
}

testStep4() {
    visible = [];
    invisible = [];
    step4();
    assertEqual(visible, [4, 15, 17, 19]);
    assertEqual(invisible, [7, 10, 11, 20]);
}

testStep5() {
    visible = [];
    invisible = [];
    step5();
    assertEqual(visible, [8, 13, 16, 18]);
    assertEqual(invisible, [4, 15, 17, 19]);
}

testStand() {
    visible = [];
    invisible = [];
    stand();
    assertEqual(visible, [4,10,12,18]);
    assertEqual(invisible, [9, 7, 8, 14, 15, 13, 11, 17, 16, 21, 20, 19]);
}

integer listsAreEqual(list a, list b) {
    integer aL = a != [];
    if (aL != (b != [])) return 0;
    if (aL == 0)         return 1;

    return !llListFindList((a = []) + a, (b = []) + b);
}

// JR Testing Ends Here

default
{
    state_entry()
    {
       llSetTimerEvent (0.0);
       num = 1;
       running = FALSE;
       stand();
       llSetLinkAlpha(1, 0, ALL_SIDES);
    }

    link_message( integer sender_num, integer n, string str, key id )
    {
        if (str=="stp")
        {
            num = 1;
            stand();
            llSetTimerEvent (0.0);
        }
        if (str=="fwd")
        {
            num = 2;
            llSetTimerEvent (0.3);
        }
    }

    timer ()
    {
        if      (num == 2){ num = 3; step2 ();}
        else if (num == 3){ num = 4; step3 ();}
        else if (num == 4){ num = 5; step4 ();}
        else if (num == 5){ num = 2; step5 ();}
    }

    touch_start(integer ignored) {
        testStep2();
        testStep3();
        testStep4();
        testStep5();
        testStand();
    }
}

I think you can see that all this has been straightforward. It was somewhat tedious, writing each test, but it didn’t take very long, and all the tests run now.

I did make the same mistake two or three times: I forgot to list my new test inside the touch_start(). Most times I noticed, but the last time, on testStand(), I was on a roll and forgot to make the test fail before making it work. This is a mistake that I make fairly often, and sometimes it will bite me. This time I got away with it. Some testing tools automatically find all the functions you write named “testXXX”, but there’s no good way to do that in LSL.

This is the first time I’ve ever gone for such extensive testing in an LSL program. I think that I should develop the habit of proceeding this way:

1. Decide on a new test;
2. Think of its name;
3. Put a call to it into the main test code (the touch_start() in this case);
4. Write the test;
5. Make it compile;
6. Watch it fail;
7. Make it work.

One way or another, I should find a way to avoid making this mistake. That’s kind of a general practice of mine. When I make a mistake, which is all the time, I don’t just chalk it up to “Oh, a mistake,” and try harder. Instead, I look to see if there is a pattern in how I work, and whether I can change that pattern a little to help me avoid that kind of mistake.

You might be worried that if you do that, pretty soon you’ll run out of mistakes and be all perfect and everything. I’m not worried about that happening to me any time soon, so trying to eliminate mistakes still makes sense.

When there’s a problem in a script, I find that it usually takes longer to figure out what it is and fix it than it would have taken to have avoided it in the first place. There’s really no way to avoid little mistakes as far as I know. “Try harder” really doesn’t work. So for me, I save time by trying to improve what I do in little bitty steps.

Maybe that will work for you, too.

Now what?

Now our script is well covered with tests. I think it is a bit improved already, because I prefer the setInvisible() to the llSetLinkAlpha(). But more importantly, we’re in a position to look for other improvements. We’ll do that next time.

Thanks for watching … see you soon!