Compartilhar via

How my old Microsoft 8-bit BASIC coding skills caused me to fail a code review

  • Artigo

I submitted some changes to an automation script a few days ago and failed my code review. Code reviews are the mechanism we use to ensure we all adhere to coding guidelines - Pascal Casing vs. camel Casing, for instance. A peer will look over my code to ensure I'm following our agreed upon best practices. There are nothing special about the OneNote test team best practices. We simply follow the .NET framework guidelines.

Anyway, my first programming experiences were on old 8 bit computers. My first formal training came with a Commodore Pet and interpreted BASIC. With little  memory, and the slowness of the first generation of home computers, any optimizations you could discover were used. One of these had to do with variables. The value of a variable was stored in a table, and each time BASIC had to retrieve the value of a variable, it started at the beginning of the table and searched through it until it found the value it needed. Imagine you wanted to organize your tools in little bins on a wall.  Each time you needed a tool, you started at the top left bin and looked in each bin until you found the tool you needed.  If your awl was in the last bin, this means you would have to look through every bin every time you needed the awl.  Clearly, if you are woodworking, you would want awls and chisels near the beginning of your bins, and wrenches sheet metal tools near the end to save yourself time.

To a running program, this meant that if you used a variable constantly, you would want it at the beginning of that table so when the computer (interpreter) went to look up it's value, it did not have to look far through the table each time you needed the variable. A variable that was only used once or twice was better located at the end of the table. Since all variables were what we now call "global," this worked fairly well.

The mechanism to add a variable in the table was pretty simple. The first time a variable was used, it was added to the table. If this was the first variable in a program, it was the first variable in the table. A pretty simple way to order variables in an optimized fashion was to declare all your variables at the beginning of a program - think of something like this from a space based shooting game:

10 DIM SCORE, ALIENS, SHIPS

In this case, the variable "SCORE" would represent the running score. Hopefully, the player would cause this to go up repeatedly and would be a candidate for the most used variable. The number of "ALIENS" killed would be the second quickest value to find, and so on.

This also meant there could be many lines of code between the creation of the variable and the first time it was actually used. Also, memory was at a premium so "SCORE" would just as likely been "S." This made knowing what a variable was used for more difficult by seeing only a line at a time. Still, the performance benefits of this practice tended to outweigh the readability of the code, and most books and articles I saw recommended putting all variables at the beginning of the code.

Fast forward to my code review.

I had a method like this:

Verify()

{

int itemCount=0;

//code

//code

//do something - about 40 lines worth of code, etc…

itemCount = GetCountOfItems();

//now verify my count is correct...

}

And I was gently corrected to change my code to this:

Verify()

{

//code

//code

//do something, about 40 lines worth of code, etc…

int itemCount=0;

itemCount = GetCountOfItems();

//now verify my count is correct...

}

With the obvious change being to move the instantiation of the variable to be just before its first use. No big deal - I made the change, resubmitted and passed. My old 8th grade programming skills let me down, though. The biggest irony here is that Commodore, the machine on which I learned to program, used Microsoft BASIC.

Just a day in the life of a tester.

Questions, comments, concerns and criticisms are always welcome,

John

Comments

  • Anonymous
    November 06, 2008
    PingBack from http://mstechnews.info/2008/11/how-my-old-microsoft-8-bit-basic-coding-skills-caused-me-to-fail-a-code-review/

  • Anonymous
    November 06, 2008
    Why not: int itemCount = GetCountOfItems(); ?

  • Anonymous
    November 06, 2008
    Is "pass/fail" terminology for code reviews typical on your team?  We routinely do code reviews as well, but I never hear people say they "failed" a code review.  In fact, it's pretty rare that a code review doesn't result in some suggestions for improvement. Regarding the particular coding pattern - I still find myself sometimes reverting to old habits of declaring variables at the top of a routine, instead of at the point of use.

  • Anonymous
    November 06, 2008
    The comment has been removed

  • Anonymous
    November 07, 2008
    BASIC sometimes tokenized variables (or truncated them to the first couple of letters) so I don't know you were saving much memory on abbreviated variables. If you really want to go back to the good old days, there are emulators written for these old systems.  I've seen one for the TI-99 computers -- it's very good.

  • Anonymous
    November 07, 2008
    Hello Seth, I'm still "active" in the 8 bit world.  I have my TRS-80 Color Computer still set up and play with it now and then.  I even wrote a short program on it to use here at work earlier this spring - some simple math analysis which could have been done with Excel, but I wanted to use the TRS-80 just to say I did.  And I've worked with MESS as well.  Those 8 bit machines may have been feeble, but they sure were fun. The TRS-80 implementation of Microsoft BASIC used only the first two letters of variable names.  I'm not sure what the TI did (and did it have a 16 bit processor?). John

  • Anonymous
    November 07, 2008
    I tend to declare my variables at the top for clarity and readability. It means that if you're wondering where variable x came from your eyes can jump to the top of the method and check the method parameters and the top of the method for any local declarations. To me declarations in the middle of a function can get lost in the noise and are a little more difficult to spot. <return type> <method>(<parameters>) {    <local variables>    <code>    <return value> } If you're keeping your methods short and simple it probably doesn't matter one way or another, but I don't know if it's fair to say one way is wrong unless it's for purposes of consistency with the rest of the code base.

  • Anonymous
    November 10, 2008
    I don't remember too much about the TI hardware.  But the cool thing about TI Extended Basic was that it had specific functions for handling moving graphics -- with "CALL SPRITE" you told it the direction, speed, starting point, shape, color of the object, and once you set it moving, the graphic would keep moving until you changed it.  There were other calls which would identify when two sprites ran into each other, which accommodated writing many variants on asteroids and space invaders.

  • Anonymous
    November 30, 2008
    I understand why chain-calling methods in tests is generally bad. But the question wasn't really about that. Your original code was:  int itemCount=0;  itemCount = GetCountOfItems(); That is, you separated variable declaration and assignment, and initialized the variable where it wasn't strictly necessary. I wondered whether there was any specific reason behind this or not (in part because in our own code reviews, I'd certainly mark such a code if I saw it).

  • Anonymous
    December 01, 2008
    The reason I decided to declare the variable and assign it 0 (instead of calling the method to assign it) was to be consistent with our guidelines.  This is a trivial case where it is overkill - but the more involved instantiation code makes the reasoning more clear. We could probably come up with a set of rules that takes simple cases like this into account.  But the more rules you have, the more complicated it gets - even if the rules are designed to simplify things... John