It’s time to cleanup code…

Now that the programs run as they should…. do they really run as they should?

Well, not fully.

I’ve still some screen functions that have no code and a lot of functions that relate to directories that just trace their use and exit. I still have to write the code to print on PDF….

Since the accounting lady wants to have a demo of the program, it’s time to have a look at the screen related code.

When a popup windows appears on screen, sometimes it is cleared on close, sometimes it is not. Looking at the code I found that the window can “close” in different modes, and which one to use is decided by the current time… It is like if you have one SaveScreen() and 3 possible RestScreen(), closing the window with 3 different visual effects.. no problem here: from these 3 functions I call directly RestScreen()… not visual, not moving, not fancy but it works.

I usually develop in a command line interface environment. I use vim editor… To be able to analyze compile errors I use a command prompt windows that I can scroll up/down. But in this way when the program starts I have to move the scroolbar to make it visible. I need to add a SetMode( 25, 80 ) somewhere.
The first idea is to add this function call to every executable…. there are already several lines that are repeated at the start of each executable… Then I decided that it was not a smart move, to create yet another duplicate code. Since this is a generic, standalone command, I added an INIT PROCEDURE in mylib.prg. INIT PROCEDUREs are run before Harbour VM gives control to the main procedure. You may have several INIT PROCEDUREs but you can’t control the order in which they are called. No problem in this case.

I also moved the check for the ROOTDIR environment variable in this function. Now it displays a message and waits for a keypress, then exits.

Starting the batch file makes the command prompt window flicker, to reduce its dimensions. Selecting a menu item that needs to call another executable makes the windows flicker again, at first it widens when the program exits and the batch file resumes, then it shrinks when the executable starts. I will set to use a 25×80 window in the properties of the batch file, so that when the users double click on the icon, the program starts with the right dimensions.

Doing a tour of the application I got errors for a missing B_DOUBLE_S variable. As you may remember the programmer took a really strange decision: instead of using #define tokens, he created variables with the same name and assigned them a value, the one taken from the include file… So now there are several PUBLIC variables just to handle static values. When I had to decompile some code due to missing sources, that strange decision turned good: instead of literal values I got the variable names… but truncated to 10 chars.

A quick series of grep and I converted the few truncated variable names to their extended counterpart. I can’t say if I spotted all of them… Since I was touching a couple of functions, I decided to do very basic changes, switching from PRIVATE to LOCAL variables.
I don’t think that this program will ever be able to be compiled with -w3 -es2 switches but when it takes so little time…

There is another step I want to make: update part of the main menu system. I can’t update the whole system because it is a very complex one. When executables call each other the old menu is selected, as if you never exited the program. This is done with a sequence of simulated keypresses, like these one:

KEYBOARD Chr( K_ENTER ) + Chr( K_UP ) + Chr( K_UP ) + Chr( K_UP ) + Chr( K_UP ) + Chr( K_UP ) + Chr( K_UP ) + Chr( K_UP ) + Chr( K_UP ) + Chr( K_UP )

It’s strange, it works, don’t touch it !!!

What I want to change is what happens next, after the menu function returns the value of the menu option selected by the user. Actually it is something like this:

IF menu > 1 .AND. menu < 8
   ONDO( menu1, NIL, 'M2()', 'M3()', 'M4()', 'M5()', 'M6()', 'M7()' )
ELSEIF menu = 13
   IF File( 'external_module.DBF' )
      ErrorLevel( 1 )
   CASE menu = 8
      ErrorLevel( 3 )
   CASE menu = 9
      ErrorLevel( 4 )
   CASE menu = 17
      ErrorLevel( 17 )

Every time that I need to understand which source file is related to a menu, I need to trace the value of menu variable, check if it is in a range, count the onDo parameters or look at the DO CASE.

So I’d like to streamline this code, removing the IFs, the CASEs and make it self documented. For example, something like this:

ONDO( menu, ;
  myExit(1), ;  // 1 - daily load
 'M2()', ;      // 2 - print totals
 'M3()', ;      // 3 - create new client
 'M4()', 'M5()', 'M6()', 'M7()', ;
  myExit(3), ;  // 8 - Banks - runs exe05
  myExit(4),.., myExit13(), ...,myExit(17))

And of course adding the two functions:

PROCEDURE myExit13()
   IF File( 'external_module.DBF' )
      myExit( 1 )

PROCEDURE myExit( nValue )
   ErrorLevel( nValue )
   RETURN  // never executed :-)

Now the various executables run fine, and the screen is always clean as it should be.

In the meantime I exchanged several emails with the accounting lady. She told me which menu options she has never used and will never use again. There are many, and I will save some time. I could have asked this question before…
But unfortunately she also told me that several menu options I disabled are actually used, only for specific cases, but used… so I enabled all of them. One of the features let the user choice a color combination. I had to create a couple of functions but it now works…. just to show that there is something still not good about colors.

I noticed that function CLS(), used to clear the whole screen and paint a background was called several times in the source code. Its color parameter is always a PUBLIC variable, set at startup depending on the color combination chosen by the user. Sometimes the variable was passed as is (a numeric value), sometimes was converted to a color string. It was quite strange. A grep on the decompiled code returned a PROCEDURE CLS with italian variable names: the function has been rewritten keeping the same name!

Looking at the procedure I discovered that for printing on screen it uses the original Funcky PRINT() function and as color parameter passes … the PUBLIC variable! The color parameter of CLS() is not used – converted or not, it is just discarded and the numeric value of the PUBLIC variable is used.

My implementation of Funcky PRINT() is very basic. It lacks color handling and the fifth paramenter, lenght, is wrongly implemented. Infact with this fifth parameter you can pad short strings or trim longer ones. PRINT() also handles printing long strings that spans multiple lines. In CLS() code, there is a single PRINT call that spans 2000 char, the whole 25×80 screen!
Before implementing the fifth parameter, a grep reveals that the function is called with the fifth parameter only in 2 cases: in CLS() function and at program startup, with lenght set to 80 and string to be printed set to one space: used to clear one line.
All the other calls are for cosmetic stuff and without the lenght parameter.
So I decided to change CLS() function to not use PRINT(): I split the 2000 chars string in 80 chars chunks and printed them, one each line.
I kept the “wrong” implementation of the fifth parameter and I used it to replicate() the string to be printed…

There is still to implement colors. I imported NTOCOLOR() function from hbct to convert from color number to color string. Now the screen is properly displayed, with proper colors and proper backgrounds.

I know there is still a lot to be done (printing, directory related functions, error handling) but it is definitively time to show the porting to the accounting lady. The screen doesn’t mess up, the colors and the background are the expected ones, just the fonts can’t be changed… for the moment… ;-)

Now tt’s time to book some time and show the progress to the accounting lady.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>