Did we meet before?

I should feel more relaxed now that I demonstrated that the program builds, can run and, best of all, can print. But I’m not.

The original program had 16 executables. One is the main menu that I already compiled (but not fully tested), one is the Utility menu, that was merged into the main menu, but only for a couple of operations.

Now I think I face the biggest problem: one by one merging all the other executables source code. Now that the basic functions are working I think that it is better to work on the biggest external program, the one the accountant spend the most part of their day: the form to load data in the db.

This form is composed of several sub-windows, each with its own way of work, several function keys always active, tons of lookup tables, popup windows… it is a real pleasure look at the lady loading data typing on the keyboard at light speed, TABs, cursor keys, function keys: she knows the exact sequence to type.

I decided to split the job in two parts. In the first I will compile the program as is, like an external program, just to see if it compiles and, expecially, links. The second is to merge the code into the main one.

As done before, I hand converted the blinker style .lnk file to Harbour .hbp file. I then added the libraries and tried to compile. There was one error in compiling, a RETURN statement nested in a BEGIN SEQUENCE that I remmed just for testing, and several missing functions. I counted 7 missing functions and looking at how they were used, some of them looked complex. Time to decompile again. I was able to recover all of them and I added it to the applib.hbp file. I alse recovered a couple of functions I had to rewrite from the main menu executable.

I had to iterate the process twice since the recovered functions called other functions I hadn’t recovered yet. Finally both main executable and this new one compiled and linked without errors.

Testing the new executable is very easy: it just needs to be run with its current directory set to the client/year subdirectory. Simple. The program started with no problem and it seems to work: I can input data, lookup table appears, all that strange popup just keep popping up but I don’t know what to write in there. Just one note: it seems that some popups doesn’t restore the underlying screen when they close. I know that some windows related functions are just stubs, empty functions with no code. I will check that later.

One more thing to note is the RETURN inside a BEGIN SEQUENCE. I don’t know why it was allowed in Clipper 87 and it is no more. What makes me feel a bit worried is that BEGIN SEQUENCE is on line 300, the RETURN statement on line 604, the END SEQUENCE on line 1568! With a nested BEGIN/END SEQUENCE and several BREAKs. It will be a long night the one I will work on this.

Terminated succesfully the first part, I started the second, merging the code. Well, actually I added the new hbp files to the old. And compiled forcing a full rebuild.

I got 51 (FIFTYONE) duplicate functions! Until now I was fighting for missing functions. Now I need to fight with duplicate functions!

I don’t know why there is this situation in the code. But as I already said, the application library is without sources, and it is the only lib. Probably the programmer didn’t know how to create a lib. Or had some other obscure need to duplicate them.

These errors messages are like an alarm ringing in my head: why so much duplicates ? Are I really sure these are real duplicates?

Let’s check.

The first duplicated function reported is lockok(). I open the 2 source files and discover immediatle that no, they are not equal. Worst, a grep reveals that this function is defined in THREE different source files, and used in 7!

The second duplicated function is a callback function of a browse. Each one is defined and used in the same source code, so I could simply add a STATIC in front of them. In no other source file there is their name.

But for the first one, what to do? The only “easy” solution is to change the functions name with a prefix of the executable they were from, info that I need to extract from the .lnk files. As I said the function is defined in 3 files and this means there are 3 executables involved. So, it is easy to change the name of the functions defined and used in the same source, but I have to search for the other 4, which executable they are linked in.

This is the most important program and I have to solve 51 duplicates. In the process I need to solve more, like the first one that I saw is present in 3 files. But what will I find next, working on the other executables? Will I find dozens more duplicates?

Should I keep the executables distinct and find a way to set the working directory? It can be as easy as adding a memowrit() to save the working subdirectory and a memoread() to read it back…

I have to think about it.

Measure, refactor, measure, refactor, measure… stop… revert

As I said already, I like refactoring code. It’s an activity that is avoided by a lot of programmers. The first excuse is that it usually takes time, a lot of time. The second is that code works, it just works so no changes are needed and it is possible to introduce new bugs refactoring. The third is that there will be no real advantage to streamline the code.

I don’t want to do a post on why refactoring is important but I just want to reply to the 3 most common excuses:
1) refactoring should be done during code development and not when the product is completed
2) tests should be in place to verify how the code works before and after the refactoring
3) it really really depends. After a refactoring, code can be more clean and easy to read. Snippets of code repeated in various parts can be refactored to a function and be present only once. Refactoring is not specific to have quicker code but it is possible, as a side effect or when set as a goal.

In the previous post we had a look at a function that was coded, well, not properly.

FUNCTION record_lock( ..., wait, ... )
   PRIVATE row,col,time,video

   row = Row()
   col = Col()
   time = wait
   video = SaveScreen( 21, 0, 23, 79 )
   IF RLock()
      csrput( row, col )
      RETURN .T.
   ENDIF
   DO WHILE .T. // Loop for RLock() retries

Also the code in the loop needs some refactoring, but it is rarely used so it’s not a problem. Also the code shown here is not really a problem, since it correctly works, but when you call this function a lot of times, there is some overhead that we may avoid.

My refactoring:

FUNCTION record_lock( ..., wait, ... )
   LOCAL row,col,time,video

   IF RLock()
      RETURN .T.
   ENDIF
   row = Row()
   col = Col()
   time = wait
   video = SaveScreen( 21, 0, 23, 79 )
   DO WHILE .T. // Loop for RLock() retries

I timed the printing procedure I used as a test in the previous post, where the function is called 15600 times, before and after the code changes. From 4.85 to 4.15 seconds (timing averaged on several tests). It’s just 0.7 seconds shaved, but it’s about 14.5% quicker now. For a 2 minutes work.

I wanted to perform another test: opening the files in exclusive mode. I changed the flag used to open the database files and removed the test in the printing loop.
The average time dropped to 3.32 seconds, a saving of 31.5%
Just for a couple of changes!

I did measure before doing any changes: after the first and simplest ones I got a 14.5% speed increment. Opening the files in exclusive mode I saved 31.5% of the time but I now have to change code elsewhere because now if another instance of the program is run, it won’t work properly. Infact I opened all the files in exclusive mode but instead I should open a couple of them in exclusive and the others in shared mode. This means adding lines, keep them in order as the program expect them, fully test that the program doesn’t break if a file is found opened by another instance of the program…. and unfortunately it breaks.

The code is almost all multi-user ready (files opened in shared mode, records locked, etc) but the programmer that sent the code to me said that it shouldn’t be used in multi-user. Don’t know why, anyway these last changes break the program.

Revert the last change, back to shared mode.

I imagine it wouldn’t be a problem to wait 4.15 seconds instead of 3.5 seconds when this job took hours.

PS: it’s really difficult timing code snippets on a multi tasking operating system working on a multi core cpu, when you can’t control exactly what other programs the pc is running in that moment. Anyway, I removed the GOTO RecNo() line and I saved another 0.40 seconds.

Printing for tax reports part 2

We are looking inside the printing code to understand why the report printed on a file has several blank lines, spaces, etc. Two functions are called inside the loop and the first is not the problem: removing it the result doesn’t change.

The loop is simple, it just prints the data, using just one lookup. But just before looping it updates a field in the current data base row…

IF record_lock( FORCE )
   REPL FIELD->FIELD_1 WITH iif( value = 1, FIELD->FIELD_2, variable )
   GOTO RecNo()
ENDIF
UNLOCK

I think there are some flaws in this code. The first one is the GOTO RecNo() that should be not necessary. The other one is more philosophical: here the program is printing the tax filing reports and instead of locking one record at time, I’d lock the whole file, to avoid that someone, during the 2 hour period the printing needed, could change the data. Working on a data file exclusively should speed up the job but at the time it didn’t matter: the dot-matrix printer was the slowest in the chain.

Anyway, I went to the record_lock() function, that was decompiled. Decompilation doesn’t change instruction order: compiler optimizer can. But I don’t even know if Clipper 87 had one.

I opened the source file, searched the function and… oh…. code is… is… a mess !

Just a little snippet, the first few lines:

FUNCTION record_lock( ..., wait, ... )
   PRIVATE row,col,time,video

   row = Row()
   col = Col()
   time = wait
   video = SaveScreen( 21, 0, 23, 79 )
   IF RLock()
      csrput( row, col )
      RETURN .T.
   ENDIF
   DO WHILE .T. // Loop for RLock() retries

What’s wrong here? Well, strictly speaking, this code has been working for 25 years so it can’t be “wrong”. Would you accept this code from a programmer working for you? I won’t.

Not when this function is called in a “tight loop”, looping, in my test case, 15600 times. The function calls 15600 times Row(), 15600 times Col(), assigns 15600 times the time variabile and 15600 times saves a block of the screen. And if the RLock() works at the first try, 15600 times the cursor coordinates are reset… but… where did we move the cursor?

I got it: Maurizio already warned me in the comments that csrput(), a function from Funcky that I rewrote, was not correct.

My version:

FUNCTION csrput( y, x )
   @ y,x SAY ""
   RETURN .T.

Maurizio version:

FUNCTION csrput( y, x )
   SetPos( y, x )
   return nil

Actually Maurizio was right. My version of csrput() did addeall that empty lines in the printing. Since the cursor was always reset to the same position, the number of empty rows was fixed, and a form feed added.

Problem found. Compiled, linked, run a new print test. Now the printing seems ok, except for the totals. Imagine a report “framed” by a 76 columns * 66 rows box printed with | and – symbols. Also the columns are divided by a | between them.

At the end of each page a subtotal is printed. I don’t know if it is the total of the page or the total of the values printed so far, the important is that when opening the printed file I got a strange result.

This is how it looks in vim:
Errore_stampa_totali

Looking at the source code, the program prints a line with code similar to this:

@ PROW()+1, 5 say "|            |            |"
@ PROW(), 6 say value PICTURE __PICT

@ PRow()+1 means to print on the following line, forcing the printer to go up one line. @ PRow() means to print on the same line… 6 is > of 5, but 6 is < of PCol(). Something strange here… but, hey, what are that strange blu symbols in the file?

Think again at the dot-matrix era. Or better, at the time of the type-writer. How did you write some letters in bold? Or underlined? You just shifted the roll to the right and typed over the previous letters to have a bold result, or typed a sequence of minus symbol after rotating the roll a bit. Actually, you over-typed. And here is the same.

Dot-matrix printers have 2 different commands: one tells the printing head to go back at column 0 (carriage-return) and the other to move the paper one step (line feed). There are many other commands, like form-feed to go to the next sheet of paper (^L) and the ones to set compressed or expanded fonts (or CPI, characters per inch). ^L is present in all reports, font selection just in a few.

So now it should be clear which was the trick used by the programmer: write one line of the box, carriage return (^M), over write the values, then ^M^L, not shown in the image since it is the standard EOL.

When I will write the converter to create a PDF from the printed file I’ll need to handle these control characters.

I think that the printing test is now completed. Test-printing to a file works with very little changes to the printing source code (just one line added) but for the real changes more code is needed.
This test also shown that some library code needs a good refactoring, something I will do in the next post.

Printing for tax reports

I’m not an accountant. I don’t know how to keep a journal of a company. My program, a mix between ERP and CMS, just prints the invoices then exports the values to be imported into the program I’m trying to port. This is the program with the official financial data. And the data from this program is used for tax reports.

The program is – as you know – Clipper 87 code, and it was written to be used with dot-matrix printer: they were the printers available at the time! And for several years the only way to print some tax reports was using dot-matrix paper: you had to buy the paper and to bring it to an office and have it “stamped” with a progressive number, so that it could be visible if, in case of a control, a paper was missing.
Unfortunately I don’t know the details.

I just remember the accounting lady reserving a full day to ready and print these reports: nobody could enter her room and she was carefully attending at the printer and checking that the paper didn’t jam.

Nowadays, you don’t get this kind of paper anymore. But the program still uses it. The solution in the last years was to export data from the program and import them into the new version, that I said in the first post, is written in a new language and uses SQL. And as I said the new version lacks some features and… you know, I’m doing the port just for this reason.

By the way, you can now print on lasers, of course. And you can use A4 paper, but you still need to have it stamped. When you get the paper back, looking at the box, you can see that page 1 is at the top, 2 is the second, all with the stampint towards you. To have the reports printed correctly, you must use a laser printer that can use the paper stacked in that position. And the first time you will surely discovered the problem when it’s too late. The lady had to revert hundreds of pages, since her laser prints on the down-facing size of the paper.

I have the program start and could create the indexes for a workarea. Now it is time to check one of the most important reason to port the program to Harbour: printing.

Looking at the menu, I choosed one option, the one that prints all the financial movements of one year. I don’t know if it is the one to print on the stamped paper but it is long and… it prints something.

My hope was to find that an external library was used, so that I could locate it or a clone in open source, or rewrite it myself. Instead, plain Clipper 87 commands are used:
SET DEVICE TO PRINTER
a series of @ SAY
EJECT
SET DEVICE TO SCREEN
as needed. Everything is plain code, neither an internal library to easy form feed, headings, footers or subtotals is used. Just plain Clipper code. This means that I have to check all the printing functions and apply the same changes to all… probably I will refactor a bit, but I first want to understand clearly how this work and, as I already said, I’d like not to change printing code too much.

Since I never used this kind of printing method, I asked harbour-users newsgroup. Daniele replied with a good answer: set the printer to a file, let the program print, open the file and convert to PDF. While reading the file, look for special chars, the ones used in dot-matrix printers to set character spacing, font width, eject, etc.
I remember to have written some code for this job when I was porting the ERP to Harbour, to create a rendering engine external to the main program, but in the end I converted my library to output to Win32 first and then to haruPdf.

In this case, my idea is to create the printed file using a temporary file name and then call a function to convert it to PDF, without calling an external program (except the PDF viewer).

Let’s test Daniele idea. The diff is quite simple, isn’t it?

The little diff to set printing on a file

The little diff to set printing on a file

Compile, link, run, select the function from the main menu: the function starts with a form asking for from-to filters pre-filled with first and last values present in the database. Confirmed the printing, the program thinks for about 10 seconds and then a message tells me the printing is completed.

With a command prompt I went to the program directory but I didn’t find the file. Current directory is another!! So I went to that directory and found the file. Opened in vim: the heading was ok, the first data line was ok, then 10 empty lines, then one line with some spaces but no data, then the second data line, in a loop. What is happening? Let’s have a look at the code!

The code uses the variable row to keep track to which line to print and it is correctly upped by 1 correctly. There should be another reason.

In the loop there are two functions called: the first one was the emergency button for the tax reports prints: pressing space during printing would stop the printing, so that you could un-jam the paper, if needed. A menu appeared and you could resume when ready. Back at DOS time, the printing was “online”, with the program waiting for the line to be ouput before executin the following instruction. If the printing took 2 hours, the program was locked for two hours!
This function checks inkey() but if it founds nothing, nothing is done with the screen. This function is outdated, there is no reason to keep it so I deleted from the printing loop: one call less.

Then there is another function call. In the next post I will explain which was the problem and other problems found in the printing code.