Intention and Code Improvement

The Piano of Damocles is one piece of my long-delayed Engines of Destruction series. It is an upright piano that hangs in the air over someone’s head, following them around if necessary. At my command, it can drop from the sky, hit them on the head, and explode into bits. True art, I’m sure of it. But there’s a problem …

The Piano of Damocles

A Problem …

Today I found a problem in the script for the Piano. When the Piano rezzes, it senses around for all the nearby avatars. The script just takes the detected names from the sensor, and puts them onto menu buttons. Then when the menu button is pressed, it takes the name and puts it into the sensor for the Piano’s following logic. The Piano goes and hangs over their head, awaiting further orders.

There was someone in NCI Kuula with a very long name, something like Arglebargleveeblefetzer Overthruster. It turns out that a menu button can be at most 24 characters. The result was that the script tried to put “Arglebargleveeblefetzer Overthruster” into the button and rather than do something useful, LSL gave a message and terminated the script.

The Cause …

Our mission is to fix the script. Ideally we’ll improve it a bit in the process. Here’s the code with the problem.

    sensor(integer num) {
        integer i;
       if ( num > 10 ) num = 10;
        list menuList = [ "vis", "invis" ];
        for ( i = 0; i < num; i++ ) {
                menuList += [ llDetectedName(i) ];
        }
        llDialog(llGetOwner(), "Follow", menuList, channel);
    }

What’s happening is that we add all the names, including the long one, to the menu list, and then when we do the llDialog(), one of the button strings is too long. There is some other code that will be affected as well:

    listen(integer channel, string nomine, key id, string msg) {
        if ( msg == "vis" ) {
            visible();
            sense();
        } else if ( msg == "invis" ) {
            invisible();
            sense();
        } else {
            avatar = msg;
            state following;
        }
    }

Here we are taking the string from the button, and setting it into the “avatar” variable, and going to the state named “following”. The “following” state takes the string in the “avatar” variable and uses it in a sensor to follow the given person around.

Some Thinking …

There are a number of ways we could solve the problem, such as truncating the name to put it into the menu, passing that truncated name on to the “following” state, and then sorting things out there. That seems wrong to me. I think this long name problem starts in the sensor code above and it should be dealt with entirely in this state.  So what we need to accomplish is to get a short name for the menu, and then turn that short name back into a long name to put into the “avatar” variable. The code we have doesn’t express that intention very well. Let’s try to express the intention first.

Expressing Intention …

 

    sensor(integer num) {
        integer i;
        if ( num > 10 ) num = 10;
        list menuList = [ "vis", "invis" ];
        for ( i = 0; i < num; i++ ) {
                menuList += [ getShortName(i) ];
        }
        llDialog(llGetOwner(), "Follow", menuList, channel);
    }

    listen(integer channel, string nomine, key id, string msg) {
        if ( msg == "vis" ) {
            visible();
            sense();
        } else if ( msg == "invis" ) {
            invisible();
            sense();
        } else {
            avatar = getLongName(msg);
            state following;
        }
    }

… adding the new functions:

string getShortName(integer i) {
    return llDetectedName(i);
}

string getLongName(string shortName) {
    return shortName;
}

It’s a Refactoring …

Note that this change makes no difference to the program. It is still using exactly the same values, even for a long name like Argle’s. We have changed the way the program design is expressed but we haven’t changed how it functions. An operation that changes design but not function is called a “refactoring” and the idea is to make the design better, or more clear, or more ready for change. This time I had in mind making the new ideas clear and preparing the code for  the implementation change.

 

The Piano Falls onto oo Magic

Now we can proceed in two stages for safety. For sure, we know that we want getShortName to return only 24 characters of name or less. We can do that with the llGetSubString() function, inside the getShortName() function.

Shorten the Name …

 

string getShortName(integer i) {
    string longName = llDetectedName(i);
    return llGetSubString(longName, 0, 23);
}

Now the getShortName function makes sure that long names are cut to 24 characters (0 to 23). You might be wondering why I stored the name into a string variable “longName” before truncating it. I did this for two reasons. First, I think it is a bit more clear and easy to read that way. But more important, I expect that we’ll need the name in its long form, to do something about getting it back. 

So what will happen now if we see anyone with a long name is that the name will be shortened to fit the button … but if we press the button, that short name will be used to feed the “following” sensor, and we won’t find the person to follow them.  That’s fine. We have two improvements to make,

1. Make the code not do anything bad on a long name;
2. Make the Piano actually follow people with long names.

We have accomplished the first of these two improvements. The code is a bit more readable than it was, and we are set up to make the next change easily. How shall we do that?

By the way, I really haven’t thought much at all about just how to do it, and certainly not while I was doing the work shown so far. One advantage to working in small steps with this “intention-revealing” code is that I don’t have to keep so many things in my tiny head all at the same time. This is good!

But I’d better think now. What shall we do? How about this. I’ll save the long names in a separate permanent list, and then in the code for getLongName from shortName, I’ll search the long names for one that matches the button up to the length of the button, and use that. 

Now maybe you see a potential problem here. What if two people have names that match up to 24 characters but not after that? Or what if we have two short names that match up to a point, such as Mary Jones and Mary Jonesette? Might we not accidentally convert and find the wrong one? I’m inclined to ignore the issue of two long names forever. But I’d like to have the Mary Jones / Jonesette thing dealt with. Either way, though, having thought of the problem, I’m going to do the simplest thing I can think of first, namely make it work for the more normal cases. Then I’ll look at what I’ve got and decide what, if anything, to do next.

So I need a new list, longNames, that gets initialized in the sensor along with the menu list, and that we add the long names to.

Save the Long Names …

 

list longNames;

string getShortName(integer i) {
    string longName = llDetectedName(i);
    longNames += [longName];
    return llGetSubString(longName, 0, 23);
}

OK. Now I’m creating the longNames list and putting names into it. The code compiles and runs at this point. (Remind me to talk about testing later on, though, and other quality issues.)

Now all that’s left is to take the short name that goes into the getLongName function, and look it up in the longNames list and return it. I’m going to do that in two steps, two looks at the list. First, I’m going to go through the longNames list looking for an exact match to the short name I have. If I find that, I’ll return it. If I don’t find it, I’ll go through the list again, looking for names for which my short one is the prefix. That way, if the list contains Mary Jonesette and then Mary Jones, we’ll find Mary Jones correctly. 

This will be a bit tricky. I’m tempted to do even this in two steps. And whenever I feel fear that I might make a mistake, I try to choose the simpler course. So first I’ll do the short name to long name match, and then add in the exact match ahead of it. Here goes the first bit:

Look Up the Long Name …

 

string getLongName(string shortName) {
    return lookUpLongName(shortName);
}

string lookUpLongName(string shortName) {
    integer numberOfNames = llGetListLength(longNames);
    integer name;
    for ( name = 0; name < numberOfNames; name++) {
        string longName = llList2String(longNames, name);
        if ( llSubStringIndex(longName, shortName) == 0 ) return longName;
    }
    return "cannot happen";
}

This works just fine. Now let’s cater for the Mary Jones problem. Notice that although I could have embedded the lookUpLongName() code directly in getLongName(), I made a separate function. That was partly for clarity but mostly because I knew I was going to add another bit of functionality to it. I plan to do that mostly with a separate function. Like this:

Look Up the Exact Name …

 

string getLongName(string shortName) {
    string exactMatch = lookUpExactMatch(shortName);
    if ( exactMatch != "" ) return exactMatch;
    return lookUpLongName(shortName);
}

string lookUpExactMatch(string shortName) {
    integer numberOfNames = llGetListLength(longNames);
    integer name;
    for ( name = 0; name < numberOfNames; name++) {
        string longName = llList2String(longNames, name);
        if ( longName == shortName ) return longName;
    }
    return "";
}

OK, this still works. I’m pretty sure it actually makes the desired changes, but I am not certain … and until I can find someone with a super long name, the way the script is now, I won’t be sure. For sure, it isn’t any worse, because I’ve been testing it as I go along.

Let’s Review …

We needed to shorten long names to go on the buttons, but to get the long names back so that the Piano would follow the person whose button we pushed. Instead of doing that all at once we did it in very tiny steps, and with the Piano still working at the end of each step:

1. We found the affected code, where we set the button and where we use the value from the button. We decided to change only in those two usages because it isolated the changes.
2. We expressed our intention, with getShortName() and getLongName() functions. We changed the script to call those but we implemented them trivially. This step was a refactoring. It changed the design very slightly but did not change the actual functioning of the program.
3. We changed getShortName() to truncate the name. This made the error go away: the script wouldn’t crash any more on a long name. But the long name wouldn’t be handled correctly.
4. We changed the script to save the long names but not to do anything with them.
5. We changed the script to look up the long name, returning the first name in the list that matched the input short name. (This implementation worked but had the Jonesette / Jones problem.)
6. We changed the script to look up exact match names, fixing the Jonesette problem.

At the end of each of these steps, we were able to test the script and it always worked. As it happens, every single of these steps worked perfectly.

Too Many Notes?

Does this seem like too many steps to you? Let me suggest why it isn’t. First, each of these things needs to be done at some point. We have to express our intention if we are going to follow the Rossini coding standard. So the four new functions have to be there. We have to truncate the name on the way in, and we have to save the long one. We have to look up the long one, and we have to look up the exact match one to solve the Jonesette problem. 

So, if we are to follow this basic approach all those steps had to be done. All I did was make each one explicit, and offer myself the chance to check at the end of each one to be sure I hadn’t made a mistake. So if there was any more work this way, it was a tiny amount … and there was no debugging or even figuring out what compiler messages meant, because I went in such tiny steps that even I couldn’t mess it up.

Just One Problem …

There is something that I don’t like about how things went. The only way I had to test this program was to run it and press its buttons to see if it worked. Since everything I changed was just  manipulating strings, in principle it should be possible to test all those changes without actually making the Piano fly. But LSL just doesn’t give us very good facilities for doing that and I had nothing in place to make it easier to test. So I just went ahead and tested manually. I don’t think that’s teh best way as a rule but it’s the best I know at this moment. 

I’ll think about better testing as I go on writing these articles.

Summing Up …

There was an unforeseen bug in my program. Maybe I should have seen it but in fact I don’t think I even knew that button lengths were limited. There’s no reason I can see to make them be that way. String in, string out. But oh well, the wisdom of the Lindens is not to be questioned by mere goddesses such as I. Anyway, there was a bug.

Rather than slam changes into the code until it worked, applying hammer where needed, I went in tiny steps. I improved the code style a bit, then added name shortening, name remembering, and name matching, bit by bit. At every moment, I was just a few typed characters away from the program working as well as it had before, or better.

I think I improved the ability of the code to express its intention, and I fixed the problem. Good Janet.

On the other hand, it’s not perfect. I can already think of names that would perhaps be better, and so on. How about avatarNames instead of longNames, for example? What about the fact that getShortName actually has a side effect of saving the long name? Are the names of the search functions good enough? What about the fact that the two search functions are almost identical?

I don’t know good answers to all those questions, and I choose not to improve further on any of them. But I try to be aware of them, if only because it helps me learn to do even better next time. For now, the Piano of Damocles is even better than before.

Thanks for reading!