Can you open my data files? part 2

In part 1 I described that the program was not able to open the database file: the filename was read from a field and its name contained spaces, it needed to be trimmed.

The code was:

file = FIELD->DBFNAME
key1 = FIELD->INDEX1
key2 = FIELD->INDEX2
key3 = FIELD->INDEX3
IF my_netuse( file, ... )
   SET INDEX to &KEY1, &KEY2, &KEY3

My solution was to trim the filename directly in my_netuse():

FUNCTION my_netuse( file, ....)
...
file := alltrim( file )

Then I discovered that the indexes were not in the backups and so I needed to create them, incorporating the first external program into the main one, as decribed in the previous post.

The indexes were correctly created but with spaces in their name. Also the index names, as you can read in the previous code snippet, are read from a database, and so also I must trim them too!

Shortly, I add a trim() for the index file name during creation and then modified the previous code to:

file = trim( FIELD->DBFNAME )
key1 = trim( FIELD->INDEX1 )
key2 = trim( FIELD->INDEX2 )
key3 = trim( FIELD->INDEX3 )
IF my_netuse( file, ... )
   SET INDEX to &KEY1, &KEY2, &KEY3

Now all files are opened correctly, dbf and ntx.

From 2 to 1 executable

I remember the accounting lady creating the indexes: from the main menu select Utility, then from the Utility menu, select Rebuild indexes and from a pop-up menu (aChoice) select the index to be recreated and finally confirm pressing a F key. The process was streamlined: I never noticed that when the lady selected Utility from the main menu, the program actually exited and a new one was loaded. The nested menus appeared without issues, and pressing exit you could go back, with the menus clearing itself and then going back to main menu executable.

I now partly understand the complex menu code: part of it is to handle screen coherency. Merging all the different programs into one should allow to simplify that code, if I will see fit.

Let’s have a look at the code of the main menu. When the user selects to open the Utility menu, an errorlevel is set and program exits. Looking at the batch file and tracing that errorlevel, I saw that it starts uti.exe. Obviously there is a uti.lnk file, a uti.prg file and several utiXX.prg files.

Uti.prg has some startup code that is a reduced version of that of the main menu source. Infact it skips some checks due to the fact that is a program that should not be called directly, but it is called with the proper working directory set. It also sets some hotkeys (SET KET), loads some default values, etc. I converted all these function calls and commands to remarks.

Uti.prg contains the Utility menu, that has about a dozen options. Some of them work on archiving data (backups, conversions from/to other programs, creating new workareas), others work on screen configuration (fonts, colors) and finally one opens the list of files that can be indexes.

I decided to disable all the options except the index one: I don’t even know if the other options are used!

So I included the following lines into exe01.hbp:

# Utility menu
src/uti
# Reindex
src/uti04

I then modified the main menu code to call uti() function and not setting errorlevel and exiting. Since the menus use ondo() that has the function names as strings, the program compiled and linked without problems.

The missing and unknown function I talked about here was used in the indexing code and I was a bit worried about it. It is used to create indexes in data root dir if the dbf file is present there and not in the subdir. The program runs correctly and the indexes are created. I didn’t check if the indexes are created in the proper directory – I’m satisfied that they are created.

Merging this code was quite easy:
– located the files to be compiled and added them to exe01.hbp
– removed all the init functions in uti.prg
– added call to uti.prg in main menu

There were a couple of issues that needed to be solved. First of all, the screen. The program didn’t remove the Utility menu when exiting: it was main menu job to refresh the screen and simulate keypresses to show the exact menu when starting, depending on the passed parameter. A lot of code full of K_DOWN, ESC, K_RIGTH that is just waiting to be removed when port completed.
Anyway, I just added a pair of saveScreen/restScreen to uti.prg. It was quick but also, perhaps, not the best choice. Perhaps adding this logic to main menu program will solve this problem for all the future imported functions.

I then did a couple of minor changes to the code. Moving all variables to local and modify some variables to fixed size arrays to variable size, filled with aAdd.
The changes were minimal, and probably a waste of time, but I wanted to go deeper into the code. I will probably refactor the code in the future because I want to add the option to rebuild all the indexes at the same time, and not only one by one: now for each index I have to press 3 keys…

Can you open my data files?

In the previous posts I described the steps to have the program compile, link and start… I don’t use the verb run but start. Infact the main menu is displayed but whatever selection I do I get one of the following: the program exits or a file open error.

The program exits when it needs to call an external executable. It is normal and expected. What is not expected is the file not found message. Why, since my_netuse() correctly opened file01.dbf?

Let’s introduce function smart_netuse()

smart_netuse() uses a lookup table to know which data files and indexes to open. Suppose you have 2 dbf: clients and invoices. Clients.dbf has indexes cli01 and cli02. Invoices.dbf has indexes ind01, ind02, ind03.
There is a table, dbftab, with several columns holding the infos related to each file, keyed by an alias (that in my case is the same of the dbf name). For example, the row related to file clients has these fields:

ALIAS: clients
DBFNAME: clients
INDEX1: CLI01
INDEX2: CLI02
INDEX3: CLI03

All the fields are C 8.

smart_netuse() is called in this way:

result := smart_netuse( action, ;
          "CLIENTS,INVOICES", ;
          "DBFTAB", ;
           exclusive, can_exit, timeout,...)

The function opens the lookup table passed as third parameter (DBFTAB) and then splits (using sub-optimal code) the second parameter and uses the values to lookup the proper rows.

The function code could have been really simple but it is not. First of all it opens the lookup table. Then it starts a loop from record 1 to LastRec() and in the loop, for each record, it extracts the first value of the comma delimited string passed as second parameter (in this case, CLIENTS) and checks if an alias() with that name is already opened. If it is not already open, if the value matches the alias field of the current record, it uses the field values to open the file and indexes. But it can’t open them and reports an error…

If I correctly read the code, I think that there are some problems to be aware, something I should keep in mind if I’ll need to add some new features. The order of the alias in the comma delimited list (second parameter) must match the order of the records in DBFTAB. I confirmed this requirement looking at the dbf and doing some greps. So, if I want to refactor this code in the future I must check if this requirement is needed in some part of the code, that may expect that the data files are opened in a certain order in adjacent workareas.

Then there is the reason of the error, a probable incompatibility between Clipper 87 and Harbour. The function does the following:

file = FIELD->DBFNAME
key1 = FIELD->INDEX1
key2 = FIELD->INDEX2
key3 = FIELD->INDEX3
IF my_netuse( file, ... )
   SET INDEX to &KEY1, &KEY2, &KEY3

Do you remember that the fields are C 8 and that my_netuse( file ) performed a simple USE &file ALIAS &file? Well, the program reported a missing file, and I should use monospaced fonts to explain:

The file not found error is for CLIENTS .DBF

Did you notice the space?

Since the error was reported by my_netuse() I went there and added a

file := alltrim( file )

to be able to open the file.

Library compiled, linked, run: now there is another error, missing index files… I’m sure you think to know where the problem is… and you are probably right, but not in this moment: the index files are really NOT present on the disk !

I used a backup to restore the dbf files and backups don’t have indexes included (they may be recreated…) so they are not present on my disk… it’s time to create them…

… but the indexes are created by a different executable…

It’s time to start integrating external source files.

Do you refactor?

In the previous posts I reformatted the source code, had a first look at the code to see that it was compilable, had a look at how the data structure is organized in the directories, compiled the first executable writing or decompiling the missing functions.

The first executable, the main one, has the main menu but only a few of the functions are internal: in a lot of situations it just sets errorlevel and exits. Well, I discovered that it does this trick also when a workarea is chosen! You are presented with a list of workareas (client/year) on screen, select one and the program first sets the current work directory, then sets the errorlevel and finally exits: the batch file checks the errorlevel value and in this case restarts the main menu. But I want to eliminate the batch file, since I want to port all the code inside one executable…

Let’s start changing code. The first step is to define the strategy of the data dirs layout, something we already spoke about in
this post. There needs to be an environment variable that points to the root data dirs and all the subdirectories (one each for client/year) must be its children.
Current code uses a mix of curdir(), curdrive() and environment variable to determine the root data dir, then uses aDir to read the directory contents into an array (predefined with 512 items), loops on the array looking for a directory and when finds one, checks to see if there are 2 files and if both are found, opens one of them and reads a value that is added to an array. When the loop is completed, the maximum length of the strings in the array is calculated and finally the user is shown a form with an aChoice.

Would you write this code yourself, in this style? No, I won’t. And I’m trying to stay away for rewriting the whole function.

At first rewriting the whole function seems the smart thing to do. The code will look modern, in your personal coding style. But I don’t thing it will be a rewarding idea. This function seems to be a self contained one, with very weak relations with other part of the code. Infact it just sets a new working directory and exits! It will be used 5, 10 times per day, and if it is a few millisecond slower than the optimum it won’t be a problem. It’s a bit ’87 style, it uses aDir() instead of the more modern Directory(), it does some loop that is not necessary but the code works flawlessy, and it has been working for the last 20 years…

There are just a few things that I feel should be done and can be done quite safely. The programmer defined all the variables as PRIVATE. There are no LOCALs and no STATICs. This must raise a big alert flag for when I will put all the code in one executable. The programmer used a lot of macro expression: I can see a lot of them in the code, and they may refer to some of these PRIVATEs.
In this specific source file (but I think the situation is common) the PRIVATE variables are defined at the top of the file, in the first procedure (implicitely defined), also if they are used only in one function!
For a6/a7 variables a RELEASE statement let me understand that the variables are not used elsewhere, but just in that function. So I moved their definition from the first function to the function they are used:

PRIVATE a7[512],a6[512],nDir

becomes

FUNCTION ChooseWorkarea
LOCAL a7[512], a6[512], nDir

I could move some more variables from PRIVATE to LOCAL because a grep returned that the name was not used anywhere else, in none of the 200 source files. The program was going to exit anyway so probably it could be safe to LOCALize some more, but I want to be on the safe side at the moment. Refactoring is my passion, but I had to stop to continue the porting job!

The code is now modified as described in the previous post. The code that set the errorlevel was removed and a RETURN was added in the nidified code.

// set the working directory
CHDIR( drive + a8[ choice ] )
// FP: instead of setting errorlevel and exiting, return
RETURN

The code works. After the user choice a workarea, the control goes back to the main menu. I can confirm that the workarea is set because the function that handles the screen painter can find file1.dbf, open it and read the company name to display on-screen.