Refactoring VII: This Series Has Legs!

It’s after dinner RL time. We just had some nice sausages and fresh veggies out on the deck. In a while, we’ll have watermelon for dessert. Meanwhile, I’ll start the next article. In this one, I’m planning to find a leg abstraction.

At the end of the last section, we had noticed that some of the step functions referred directly to what appear to be lists of leg positions from a single frame. Here are all those functions for our amazement:

stand ()
{
    list off = [LF2, LF3, LF5, RL2, RL4, RL5, RF3, RF4, RF5, RR2, RR3, RR4];
    list on  = [LF_STAND, RL_STAND, RF_STAND, RR_STAND];

    changeVisibility(off, on);
}

step2 ()
{
    list off = [LF4, RL3, RF5, RR5, LF5, RL5];
    list on  = [LF2, RL2, RF2, RR2];

    changeVisibility(off, on);
}

step3 ()
{
    list off = [LF2, RL2, RF2, RR2];
    list on  = [LF3, RL3, RF3, RR3];

    changeVisibility(off, on);
}

step4 ()
{
    list legs3 = [LF3, RL3, RF3, RR3];
    list legs4 = [LF4, RL4, RF4, RR4];

    changeVisibility(legs3, legs4);
}

step5 ()
{
    list off = [LF4, RL4, RF4, RR4];
    list on  = [LF5, RL5, RF5, RR5];

    changeVisibility(off, on);
}

In the code above, I’ve highlighted all the occurrences of leg positions from a single frame. It’s not too surprising that most of the functions have those frames in there, since mostly a step consists of turning off the prims from the previous step frame, and turning on the ones from the current frame. Two of the functions are a bit different.

The step2 function turns on frame 2 well enough, but it turns off both frame 5, plus two other leg positions, LF4, and RL3. And the stand function basically turns off everything that it doesn’t turn on. We’ve speculated, I think accurately, that this is because the stand position can be entered from any frame, so it must turn off everything it doesn’t use. But let’s look at these in a bit more detail.

First, step2. What’s up with the LF4 and RL3? Let’s take a look at the data definitions, and I think we’ll see:

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;

I’ve put //* comments on LF4 and RL3, and on the corresponding stand values. They are the same! So what is happening in step2 is that it is turning off all the values for step5, where it normally comes from, but also turning off the values from stand that it does not use.  That code, the original we refactored, and now ours, would have been more clear had it referenced the stand version of those constants, not the step5 version. I’m going to change that list to mention the stand values, and rerun the tests. That works, and the new version looks like this:

step2 ()
{
    list off = [LF_STAND, RL_STAND, RF5, RR5, LF5, RL5];
    list on  = [LF2, RL2, RF2, RR2];

    changeVisibility(off, on);
}

That is at least a bit more intention-revealing. But why doesn’t step2 turn off the other two values, RF_STAND and RR_STAND? Look again at the constants in the list. RR_STAND is the same as RR5, which is turned off in the second part of the list. And RF_STAND is the same as RR2, which is turned on in the second call. So RR_STAND was removed for efficiency, and RF_STAND removed because it is supposed to be left on, not turned off. That leg doesn’t move on transition from standing to stepping.

So we see two points of note. First, there are only 16 leg positions in the current version, one for each leg for each step. Second, the standing position consists of leg positions selected from those 16. These two things are the reason why the stand function is so odd … it has to turn off a random 12 out of 16 legs, and why step2 is a bit odd, because it has to accomodate entry from step5, which is normal, but also from stand, which requires turning off two extra legs.

So, in setting my expectations for how this may turn out, I can expect that there will be four orderly data structures for the legs, but a couple of less orderly ones to assist with the extra turning off that is required by these special cases. We’ll keep that in mind.

For now, let’s continue to proceed in small steps. I’m going to rename the lists that are created in the various stepping and standing functions, to give them consistent and meaningful names. Here’s the result:

stand ()
{
    list not_standing = [LF2, LF3, LF5, RL2, RL4, RL5, RF3, RF4, RF5, RR2, RR3, RR4];
    list standing     = [LF_STAND, RL_STAND, RF_STAND, RR_STAND];

    changeVisibility(not_standing, standing);
}

step2 ()
{
    list leg_position_5_plus_stands = [LF_STAND, RL_STAND, RF5, RR5, LF5, RL5];
    list leg_position_2  = [LF2, RL2, RF2, RR2];

    changeVisibility(leg_position_5_plus_stands, leg_position_2);
}

step3 ()
{
    list leg_position_2 = [LF2, RL2, RF2, RR2];
    list leg_position_3  = [LF3, RL3, RF3, RR3];

    changeVisibility(leg_position_2, leg_position_3);
}

step4 ()
{
    list leg_position_3 = [LF3, RL3, RF3, RR3];
    list leg_position_4 = [LF4, RL4, RF4, RR4];

    changeVisibility(leg_position_3, leg_position_4);
}

step5 ()
{
    list leg_position_4 = [LF4, RL4, RF4, RR4];
    list leg_position_5 = [LF5, RL5, RF5, RR5];

    changeVisibility(leg_position_4, leg_position_5);
}

Cool! After a little messy stuff at the beginning, things smooth right out. We see more clearly than before that the step functions are changing from one leg position to another. This code used to think that it was turning more or less rangom values on and off, with a bit of a mnemonic character in their names. Now the code knows that it is switching from one leg position to another. We might even consider renaming changeVisibility to something like switchPosition to reflect that.  But not yet. What’s next for us? Removing duplication!

Remember that whenever we see duplication in our code, it is a hint that there is an idea lurking, waiting to be better expressed. Here, we have all those leg_position variables, most of which are used more than once.  I’m going to remove that duplication by moving the leg_position things out to globals.

This is not a move I would make lightly, by the way. Creating globals is always a problem in our code. It couples one part to another in ways that are hard to manage. But for now, we have little choice if we want to remove this duplication, and we do. Well, at least I do.

Brief digression … does it seem to you that I am just following my nose here, without any grand plan? Does it seem that I don’t know where I’m going to wind up? Well, that’s somewhat what I’m doing. However, I’m not following my untrained nose blindly. Instead, my nose is trained to detect certain things, like duplication and code that doesn’t express itself well, and having nosed those things out, I improve them. So the result is that a better program sort of emerges, like the interesting things that emerged on my desk when I was cleaning it last night while chatting with Dizzi. Cleaning my desk, cleaning the code. Both lead to a better situation without a lot of deep thought. But I digress …

OK, here goes. Moving the leg position lists out to global, we get this:

list leg_position_2 = [LF2, RL2, RF2, RR2];
list leg_position_3 = [LF3, RL3, RF3, RR3];
list leg_position_4 = [LF4, RL4, RF4, RR4];
list leg_position_5 = [LF5, RL5, RF5, RR5];

list legs_standing  = [LF_STAND, RL_STAND, RF_STAND, RR_STAND];
list not_standing   = [LF2, LF3, LF5, RL2, RL4, RL5, RF3, RF4, RF5, RR2, RR3, RR4];
list stands_plus_5  = [LF_STAND, RL_STAND, RF5, RR5, LF5, RL5];

stand () {
    changeVisibility(not_standing, legs_standing);
}

step2 () {
    changeVisibility(stands_plus_5, leg_position_2);
}

step3 () {
    changeVisibility(leg_position_2, leg_position_3);
}

step4 () {
    changeVisibility(leg_position_3, leg_position_4);
}

step5 () {
    changeVisibility(leg_position_4, leg_position_5);
}

You’ll notice that I’ve gone to my preferred spacing of the curly brackets. With these one-line functions I much prefer the closer spacing.

I think this is starting to look good now. Instead of the separately-named leg values, we have lists of values, each one representing a meaningful set of prims to make visible or invisible. Are you likeing this representation better? I am. However, there are still some issues that concern me.

There are a lot of data lines in there now, the individual items, and then our new lists. It’s a lot to take in. But if we just put the numeric values in the leg lists, would that be less communicative? In addition, while the simple leg lists are pretty reasonable, the multiple lists are not as clear as we might like.

Anyway, the script is working, and I think it is better than it was. I’ll end this report here and start my RL day. For your reading pleasure, below is the entire program as it stands, tests and all. It’ll help you see where we are and think about whether it’s all worth it. I hope it is!

The Whole Program:

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;

list leg_position_2 = [LF2, RL2, RF2, RR2];
list leg_position_3 = [LF3, RL3, RF3, RR3];
list leg_position_4 = [LF4, RL4, RF4, RR4];
list leg_position_5 = [LF5, RL5, RF5, RR5];

list legs_standing  = [LF_STAND, RL_STAND, RF_STAND, RR_STAND];
list not_standing   = [LF2, LF3, LF5, RL2, RL4, RL5, RF3, RF4, RF5, RR2, RR3, RR4];
list stands_plus_5  = [LF_STAND, RL_STAND, RF5, RR5, LF5, RL5];

stand () {
    changeVisibility(not_standing, legs_standing);
}

step2 () {
    changeVisibility(stands_plus_5, leg_position_2);
}

step3 () {
    changeVisibility(leg_position_2, leg_position_3);
}

step4 () {
    changeVisibility(leg_position_3, leg_position_4);
}

step5 () {
    changeVisibility(leg_position_4, leg_position_5);
}

// JR Implementation Starts Here

changeVisibility(list off, list on) {
    integer itemMax = 4;
    integer itemNumber;
    for ( itemNumber = 0; itemNumber < itemMax; itemNumber++ ) {
        setInvisible(llList2Integer(off, itemNumber));
        setVisible(llList2Integer(on, itemNumber));
    }

    integer offMax = llGetListLength(off);
    for ( itemNumber = 4; itemNumber < offMax; itemNumber++ ) {
        setInvisible(llList2Integer(off, itemNumber));
    }
}

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

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

// JR Implementation Ends Here

// JR Testing Starts Here

list visible;
list invisible;
list ordering;

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);
}

setupTest() {
    visible = [];
    invisible = [];
    ordering = [];
}

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

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

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

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

testStand() {
    setupTest();
    stand();
    assertEqual(visible, [4,10,12,18]);
    assertEqual(invisible, [9, 7, 8, 14, 15, 13, 11, 17, 16, 21, 20, 19]);
    assertEqual(ordering, [9, 4, 7, 10, 8, 12, 14, 18, 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();
    }
}

Thanks for reading!