The Gentle Sigh Of Despair

Sometimes things just get worse

Earth Date 2008.07.31

Posted by Rich Wheadon | Permalink



The temperature outside was 93 degrees... but that was nothing compared to the heat coming off our internal customers. Someone had been overpaid by several thousand dollars recently and heads were going to roll if something like that EVER happened again. The group responsible for payment came to IS and shared this "new" problem with us. As observed system behavior was compared to code logic we began to realize that either the problem had been around for a long time... or someone had exploited a bug somewhere recently. A few moments in meeting with the user community revealed exactly what we had expected... the system was being used in a manner contrary to how it should have been used. Furthermore, the people using the system conceded that they knew what they were doing was wrong... but they had no choice and it was our job to figure out how to bail them out.

As I took in the business reasons (or causes) for the process change which lead to system workflow abuse I noticed the users began to go over convoluted gyrations and hoop jumping they practiced daily to overcome bugs in the system design... bugs that had (until now) never been reported or even communicated to IS. As the scene unfolded I noticed the product manager working overtime to grasp exactly where the system had failed due to bugs and where the system had failed due to user process error. When the meeting ended there was half a page of action items for IS to work through.

So here's the scoop... our company has an organically enhanced (no, we aren't growing hemp in it) timesheet application that has had other timesheet applications strapped into it over the years. The original system was designed for manual timesheet entry, then upgraded to import fax timesheet entry, then upgraded to accept a VMS file import to timesheets, then upgraded to accept our internally developed timesheet portal (different technology) timesheet files, then upgraded to accept some more VMS file imports for both internally employed and external employees. Every new interface used a different set of rules for timesheet handling within the original system... woo hoo.

Somewhere along the line our finance folks had begun exploiting the system loopholes intentionally left open to allow them to ensure what is passed through would meet their stringent compliance processes. Instead of strengthening for compliance they began actually making the data bad... this bad data would flow through and at the very end of their processes they would magically turn it into good data... until one day. That fateful day someone didn't notice that a debit had gone through the system where an associated credit (cancelling out the debit) got hung up due to some rounding issues. The magical (manual) final reconciliation process failed. And no matter how much had been done to screw up the data... it was the job of IS to fix the system so it wouldn't happen again.

Here's where I came into the picture.

IS had finally worked all of the problems down to 2 major points of impact. One was the method by which finance would have to send imported timesheets and adjustments through the system to maintain support, and one where a longstanding bug would be fixed so that the reported grouping of time would be correct. My code fix for the grouping was darn near mindless and I was suprised the original developer made the code so loose... especially since the platform is Lotus Notes and anyone who develops in Lotus Notes would tell you that you can never really KNOW what the data coming through is going to be like. (Unless you wrote the whole app and have closed every loophole and no one else can change anything.)

The code was in Notes Formula Language... which makes it even more elementary. Essentially the original developer somehow assumed that a comparison between strings and numbers would work. I Simply did some encapsulation of the incoming numeric values by casting them to string... I wanted to also trap for zeros... but that request was denied.
So here's the old code:
@If(
HoldPayment = "Y";"Holding Payment";

CandType = "Inc" & (ThirdPartyRef != "Y" | TransCode != "TSX") & (InvNum = "" | InvAmt = "")
| (TransCode = "TSX" & ThirdPartyRef = "Y" & (InvNumOth = "" | InvAmtOth = ""));"Invoices Not Received";

(CandType = "Inc" & TransCode="TS":"TSA" & TotRegHours != "" & InvAmt != "" & InvAmt != GrossPayTotal)
| (CandType = "Inc" & TransCode = "ME":"MEA" & InvAmt != ExpenseTotal)
| (TransCode = "TSX" & ThirdPartyRef = "Y" & InvAmtOth != ExpensePayTotal)
| (TransCode = "TSX" & ThirdPartyRef != "Y" & InvAmt != ExpensePayTotal);"Invoices/Timecards Need Correction";

"Invoices Set to Process"
)

See where InvAmt!="" is used... that would always cause a false negative. (hee hee)

I made some simple changes to utilize variable cache and casting to text:
vIA:=@Text(@GetField("invAmt");"G");
vIAO:=@Text(@GetField("invAmtOth");"G");
vTRH:=@Text(@GetField("totRegHours");"G");
vGPT:=@Text(@GetField("grossPayTotal");"G");
vET:=@Text(@GetField("expenseTotal");"G");
vEPT:=@Text(@GetField("expensePayTotal");"G");

@If(
HoldPayment = "Y";"Holding Payment";

CandType = "Inc" & (ThirdPartyRef != "Y" | TransCode != "TSX") & (InvNum = "" | vIA = "")
| (TransCode = "TSX" & ThirdPartyRef = "Y" & (InvNumOth = "" | vIAO = ""));"Invoices Not Received";

(CandType = "Inc" & TransCode="TS":"TSA" & vTRH != "" & vIA != "" & vIA != vGPT)
| (CandType = "Inc" & TransCode = "ME":"MEA" & vIA != vET)
| (TransCode = "TSX" & ThirdPartyRef = "Y" & vIAO != vEPT)
| (TransCode = "TSX" & ThirdPartyRef != "Y" & vIAO != vEPT);"Invoices/Timecards Need Correction";

"Invoices Set to Process"
)

Although I would have liked to overhaul the logic even more... refactoring for performance and extensiblility was not an option.

I guess the message I have here is... don't make data assumptions and don't do shortcuts.... cover your rear!

Here's a real common one: You have code all over the place that checks to see if the document you are handling is a conflict... in this example we'll just delete those pesky critters...
======Sample Code Snip 1=============
' * Loop through the collection and delete conflicts
Dim workingDoc As notesDocument
Set workingDoc = someDocCollection.getFirstDocument
While Not(workingDoc Is Nothing) ' (sigh)
If workingDoc.hasItem("$Conflict") Then Call workingDoc.removePermanently(True)
Wend


Pretty simple, but i'd rather be able to say something equivalent to "if workingDoc.isConflict then workingDoc.removePermanently True" But there is no .isConflict property for a notes document. So make a function in your globalFunctions library to handle it!
Function isDocConflict( docIN As notesDocument ) As Boolean
On Error Goto erh
If docIN Is Nothing Then Error 2008, "No document to handle"
isDocConflict=False
If docIN.HasItem("$Conflict") Then isDocConflict=True
Exit Function
erh:
Exit Function
End Function

and use it..
======Sample Code Snip 2=============
' * Loop through the collection and delete conflicts
Dim workingDoc As notesDocument
Set workingDoc = someDocCollection.getFirstDocument
While Not(workingDoc Is Nothing) ' (sigh)
If isDocConflict(workingDoc) Then Call workingDoc.removePermanently(True)
Wend

What's the difference? For one, when IBM gives us a property or method to determine conflicts other than sniffing for the $Conflict field then the code can be optimized in one place rather than many. Additionally, the function will work whether you know you have a document or not... our example already made sure we had one, but the function doesn't depend on that and protects itself with a check. And even further you can add logging and any other special stuff you want to the function without having to go back through your code to upgrade.

So a 9 line function IS more than one line to check whether a document is a conflict, but that function allows predicatability & sustainability in your app. Start thinking about all of the little hacks you put all over the place and turn them into something that looks more readable where you use them. One tip... try to name your functions/subs in such a way that future releases of LotusScript won't conflict with your code. (When R5 added the Replace() method I almost died updating my software that contained my own Replace() method)

Thought I had shared this fun one...
I got tired of doing a counter = counter + 1 line every time I wanted to increment it. (would love counter++)
So I wrote a wrap for it:
Sub incrementThis( numIN As Long )
numIN = numIN+1
End Sub

more fun to use:
call incrementThis(counter)
or
incrementThis counter

And I can change the function later without impacting other stuff later. Why did I use a long? ... Because Integers aren't big enough and bite me all the time.

If you are using integers then you're lucky enough to get to extend the functions (kinda)
You will write another sub:
Sub incrementThisInt( counter As Integer )
Dim longCounter As Long
longCounter=Clng(counter)
incrementThis LongCounter
counter = Cint(LongCounter)
End Sub

now you increment your integers...
incrementThisInt counter
woo hoo, the simple things are fun too