0 Comments

You may have noticed the Crystal Reports tag attached to this post.

Yes.

Crystal Reports.

The much maligned, overly complex and generally poorly regarded report generation software that I’m sure all software developers have had to touch at some point or another.

Well, it was finally my turn to touch it. I’ve managed to dodge it for the last 10 years of my career, but it looks like my number finally came up and I needed to delve into the belly of the beast to try and figure out why it was generating incorrect output.

Unluckily for me it wasn’t even a newer version of Crystal Reports, it was an old version. For all I know the component might be amazing now, but I suspect it has its reputation as a result of the older versions anyway, so I at least knew what I was getting myself into.

Reporting Issues

I’m currently working on a legacy line-of-business application written primarily in VB6. We are in the process of (slowly) moving it into .NET (which is good), but being that this is a gradual process, we still need to maintain and support the older code, ensuring that it continues to work as it always has.

This particular application, like many line-of-business applications, has the ability to generate many different varieties of reports. Suffice to say though, they are important to the users of the software.

The reports are generated using an old ActiveX version of Crystal Reports, for use with VB6. I don’t actually know what version it is.

There was a bug logged that a particular section of one of the reports generated by the application was accumulating values between rows for a particular field.

That’s right, values were accumulating, meaning that a column in the first row would have some text in it like “Discombobulation Widget”, and the second row would have “Discombobulation Widget Frazwozzer Widget”, when it should have just said “Frazwozzer Widget”. The next row was worse, containing 3 different values jammed together. The weirdest part was, it was only happening for some of the rows, not all of them. Only rows representing data of a particular type would exhibit this behaviour, others would be fine.

This was a new thing, as apparently the report had worked correctly in the past.

Of course, the obvious cause would be that the dataset behind the report was wrong, and contained the wrong values for those fields. Something that could be dealt with inside SQL or maybe VB6 if you’re unlucky.

That was not the cause. The data was fine.

It was coming from inside the report…

Maw of Madness

The reports in the application are defined as .rpt files, which seem to be the Crystal Reports file format. The files are binary of course, so diff tools are useless for tracking changes,

Anyway, after some digging I figured out that Crystal Reports has the concept of formulas, which seem to define the calculations behind a field. Fields are what you use to show actual data in the report, and seem to be derived from the dataset supplied to the report. In the case of this report, the dataset was being supplied via an SQL query on a temporary table that the application had constructed prior to running the report.

The field that was accumulating values between rows was called “details”, which isn’t all that important.

It had a formula though, which looked like this:

stringVar sDetails;

if {@TableType} = 2 and ({Transactions.TransType} = "RE" or {Transactions.TransType} = "LE") then
(
    if IsNull({Transactions.FromText}) or {Transactions.FromText} = "" then
        if IsNull({Transactions.FromRef}) or {Transactions.FromRef} = "" then
            sDetails:={Transactions.Details} 
        else
            sDetails:={Transactions.Details} + " (" + {Transactions.FromRef}+")"
    else
        sDetails:={Transactions.Details} + " (" + {Transactions.FromText} + ")";
)
else if {@TableType} = 0 then
(
     //( //Bank Ledger
    if not isNull({Owners2.FileAs}) and {Owners2.FileAs} <> "" then
        sDetails:={Owners2.FileAs} + " - " + {Transactions.Details}
    else if isNull({Owners2.ID}) and not isNull({Creditors2.FileAs}) and {Creditors2.FileAs} <> "" then
        sDetails:={Creditors2.FileAs} + " - " + {Transactions.Details}        
    else                
        sDetails:={Transactions.Details}
)
else if {@TableType} = 4 then //Sale
(
   if {Transactions.LedgerType}="C" then
        if isnull({Transactions.FromText}) or {Transactions.FromText}="" then
            sDetails:={Transactions.Details}
        else
            sDetails:={Transactions.Details}+" ("+{Transactions.FromText}+")"
)
else if {@TableType} = 5 then //Tenant
(
    if not isNull({ID.ActionID}) then
        sDetails:={Actions.Subject}
    else if not isNull({ID.LogID}) then
        sDetails:={Log.Details}
    else
        sDetails:={Transactions.Details}
)
else
    sDetails:={Transactions.Details};


if {Transactions.LedgerType} ="D" then
    if isnull({Transactions.Paidby}) or {Transactions.Paidby}="" then
        sDetails:=sDetails
    else
        sDetails:=sDetails+"("+{Transactions.Paidby}+")";


if {ID.TextField2}="EFT" or {ID.TextField2}="JNL" then
    sDetails:=sDetails + " " + {ID.TextField3};

sDetails;

By the way, that’s not any normal language to my knowledge. Its some Crystal Reports specific script.

Regardless of what it is, it’s not good code, but it doesn’t look like it has any obvious bugs that would be causing it to carry over data for previous rows.

I’ll cut right to the chase, even though it took me quite a while to figure it out.

See that variable declaration at the top of the code block above?

stringVar sDetails;

That’s not locally scoped.

I know. How could that possibly not be locally scoped. Its right there. Its literally declared inside the script block.

As a result of it not being locally scoped, the values held within it can bleed over when the formula is evaluated for each row. That means there is a code path somewhere in those nested if statements that does not set the variable directly, and instead uses the value already there, which is the root cause of the accumulating values.

That piece of code is at the bottom, executed when the LedgerType is D. It takes the current value and just appends some additional information. When the TableTable is 4 (Sale) and the LedgerType is D, there is no code path that initialises the sDetails variable, meaning it keeps whatever value it had in it from the previous run.

The fix?

Initialise the variable to the empty string at the top and run away as quickly as possible.

Protect Yourself

Well, maybe not run away as quickly as possible.

I didn’t mention it earlier, but there were no tests for this report. I think you probably saw that coming though.

Nothing was being run that would automatically check that is contents were approximately what you would expect. Being that the report had worked in the past, this sort of test would have caught the error (which was introduced when some “improvements” were being made) very close to when the code was actually written, making it much easier to fix.

I would like to rectify the lack of test at some stage, but I know putting that sort of test in place, for the very first time, will not be easy, and will be extremely time consuming.

Testing the code in an isolated way seems to be impossible, as parts of it live in VB6 and other parts live in Crystal Reports, which seems to have a hard dependency on an actual database (actually a temporary table in the database that the application puts together to make the report work, but same idea). That means no easily stubbed out unit test.

The only way I can see to test it end-to-end would be to create a functional test that runs the application itself to create the entries needed for the report, generates the report and then checks it against a screenshot or something similarly simple and robust.

I have not yet done this, but I hope to soon.

Conclusion

Typically I like to close out my blog posts with some sort of lesson learned, or something that I have taken away from the situation that may help me (and maybe you) in the future.

I’m not entirely sure what I can take away from this experience.

Don’t trust third party components?

Sometimes variables that appear local aren’t?

Maybe the lesson is that there isn’t always a lesson, and sometimes software is just super depressing.