App for providing accounting solutions for clients

Question

I have a VB6 app that has existed for more than 7 years to which I have a few clients using it for their accounting solutions. Now my problem is that a particular client has grown so big with a 15GB database size, a ledger database table of more than 4 million rows, customer size of more than 100,000, etc.

I have code in that app where customers are debited for a flat rate after checking some parameters, for a client with a small or average database. It works just fine, but for this particular client, this code sometimes does not even complete in 6 hours. I am sincerely looking for help in solving my Do Until loop headache. Is there a way to optimize this code and eliminate the Do Until statement or make it run a lot faster?

TT1 = "GOODS%"
GClass = "10"
Shuss = "Interest Declaration"

Set rst = HConn.Execute("Select * from tblCInterest where AccID like '" & TT1 & "' order by AccID")
If rst.BOF Then
   MsgBox " There Is No Existing Account Under That Type", vbInformation
   Exit Sub
Else

Set rstd = HConn.Execute("Select count(*) as Countss from tblCINT where AccID like '" & TT1 & "'")
If rstd.EOF Then
Else
Label6.Caption = rstd!Countss
End If

Set rstG = HConn.Execute("select Balw from tblSerialCount")
If rstG!Balw = "" Then
   MsgBox " You Have Not Setup The Percentage", vbInformation
   Exit Sub
Else
End If

Set rstB = HConn.Execute("select srefno from tblSerialNo2")
If rstB!srefno = 0 Then
   MsgBox " You Have Not Setup The Parameter For Maximum Sales in a Month Per Customer !", vbInformation
   Exit Sub
Else
End If

Set rst2 = HConn.Execute("Select * from tblSerialNumbers")
DPP = Val(rst2!Int1) + 1
Trump = "INT" & GClass & "/" & Val(rst2!Int1) + 1

DAF:
Set rst2T = HConn.Execute("Select * from tblLedger where GREF = '" & Trump & "'")
If rst2T.BOF Then
Else
DPP = Val(DPP) + 1
Trump = "INT" & GClass & "/" & DPP
GoTo DAF
End If

HConn.Execute ("update tblSerialNumbers set Int1 ='" & Val(DPP) & "'")

Muka = Trump
GD = Trump
OD1 = 0


On Error GoTo ErrHandler
Do Until rst.EOF
BAL1 = 0
BAL2 = 0
PBar.Value = PBar.Value + 1
'INT
Joe = Val(Trim(Text1.Text)) / 100
Joe6 = Val(rst!CustBaL1)
Joe2 = Round(((Joe * rst!CustBaL1) / 12), 2)
'WHT
If Val(rstG!Balw) > 0 Then
TanG = "Total Tax For " & Dew2 & " " & Year(Date)
Tan6 = Val(rstG!Balw) / 100
Tan5 = Round(Val(Joe2) * Tan6, 2)
Else
TanG = "Total Tax on Interest Paid For " & Dew2 & " " & Year(Date)
Tan5 = 0
End If


Set rstB = HConn.Execute("select srefno from tblSerialNo2")
If rstB!srefno = 0 Then
Else
   Set rstc = HConn.Execute("select count(*) as Bety from tblSalesR where AccID = '" & rst!AccID & "' and Wmonth = '" & Joe4 & "' and Wyear = '" & Joe5 & "' ")
   If rstc!bety > Val(rstB!srefno) Then
      GoTo Dante
   Else
   End If
End If

Set rst15 = HConn.Execute("Select Blocked from tblClients where AccID = '" & rst!AccID & "'")
If rst15.BOF Then
GoTo Dante
Else
 if rst15!Blocked = "YES" then
    GoTo Dante
 else
 end if
end if

Set rst2 = HConn.Execute("Select * from tblClientInterest where Acctno = '" & rst!AccID & "' and period = '" & Month(Date) & "' and IntYear = '" & Year(Date) & "' ")
If rst2.BOF Then
Else
GoTo Dante
End If

Set rst15 = HConn.Execute("Select Bal101,CCode from tblClients where AccID = '" & rst!AccID & "'")
If rst15.BOF Then
GoTo Dante
Else

BAL1 = Val(rst15!Bal101)

'MINIMUM BALANCE SETUP
Set rstF = HConn.Execute("select Flopno from tblSerialNo2")
If Val(rstF!flopno) >= 0 Then
   If Val(BAL1) < Val(rstF!flopno) Then
      GoTo Dante
   End If
Else
GoTo Dante
End If

Joe3 = Round((Val(rst15!Bal101) + Joe2) - Tan5, 2)
Joe4 = Month(Date)
Joe5 = Year(Date)

BranchT = rst15!CCode
End If

If Val(Joe2) < 0 Then
   GoTo Dante
End If

'ELIMINATE ZERO BALANCE
If Val(BAL1) < 0 Then
   GoTo Dante
End If


Set rst11 = HConn.Execute("select * from tblAcctClasses where Acct='" & DataCombo1.Text & "' and Bcode = '" & BranchT & "'")
If rst11.BOF Then
MsgBox " The Client Type Does Not Exist In The Database ! ", vbInformation
GoTo Dante
Else
TT11 = rst11!ACCTCODE
TUID = rst11!ACCTCODE
End If



GOVAcct = rst!AccID
GOVRef = Muka


'INT
DBAL1 = Round(Val(BAL1), 2)
HConn.Execute ("insert into tblLedger values ('" & Replace(rst!ClientName, "'", "''") & "','" & rst!AccID & "','" & Replace(DataCombo1, "'", "''") & "','" & Year(Date) & "','" & Date & "','" & Shuss & "','" & Muka & "','0','" & Val(Joe2) & "','0','0','0','','" & Replace(PostUser, "'", "''") & "','" & Format(time, "HH:MM:SS Am/Pm") & "')")
BAL2 = Round(Val(BAL1) + Val(Joe2), 2)
HConn.Execute ("Update tblClients set Bal101='" & BAL2 & "' where AccID = '" & rst!AccID & "'")


If Val(DBAL1) >= 0 Then
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & TT11 & "','" & Replace(Shuss, "'", "''") & "','" & Muka & "','0','" & Val(Joe2) & "','" & Replace(PostUser, "'", "''") & "')")
ElseIf Val(DBAL1) < 0 Then
If Val(DBAL1) + Val(Joe2) <= 0 Then
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & C5 & "','" & Replace(Shuss, "'", "''") & "','" & Muka & "','0','" & Val(Joe2) & "','" & Replace(PostUser, "'", "''") & "')")
GOVVal = Val(Joe2)
PERFCHECK4
Else
CBAL1 = Val(DBAL1) * -1
CBAL2 = Round(Val(Joe2) + Val(DBAL1), 2)
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & TT11 & "','" & Replace(Shuss, "'", "''") & "','" & Muka & "','0','" & Val(CBAL2) & "','" & Replace(PostUser, "'", "''") & "')")
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & C5 & "','" & Replace(Shuss, "'", "''") & "','" & Muka & "','0','" & Val(CBAL1) & "','" & Replace(PostUser, "'", "''") & "')")
GOVVal = Val(CBAL1)
PERFCHECK4
End If
End If

HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & C6 & "','" & Shuss & "','" & Muka & "','" & Val(Joe2) & "','0','" & Replace(PostUser, "'", "''") & "')")


'TAX

BAL1 = Val(CustBaL1(rst!AccID))
BalTax = Val(BAL1)
BalTax2 = Round(Val(BalTax) - Val(Tan5), 2)

HConn.Execute ("insert into tblLedger values ('" & Replace(rst!ClientName, "'", "''") & "','" & rst!AccID & "','" & Replace(DataCombo1, "'", "''") & "','" & Year(Date) & "','" & Date & "','" & TanG & "','" & Muka & "','" & Val(Tan5) & "','0','0','0','0','','" & Replace(PostUser, "'", "''") & "','" & Format(time, "HH:MM:SS Am/Pm") & "')")
BAL2 = Round(Val(BAL1) - Val(Tan5), 2)
HConn.Execute ("Update tblClients set Bal101='" & BAL2 & "' where AccID = '" & rst!AccID & "'")


If Val(BalTax) <= 0 Then
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & C5 & "','" & TanG & "','" & Muka & "','" & Val(Tan5) & "','0','" & Replace(PostUser, "'", "''") & "')")
GOVVal = Val(Tan5)
PERFCHECK3
ElseIf Val(BalTax) > 0 And Val(BalTax2) < 0 Then
CBAL1 = Val(BalTax)
CBAL2 = Round(Val(Tan5) - Val(CBAL1), 2)
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & TT11 & "','" & TanG & "','" & Muka & "','" & Val(CBAL1) & "','0','" & Replace(PostUser, "'", "''") & "')")
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & C5 & "','" & TanG & "','" & Muka & "','" & Val(CBAL2) & "','0','" & Replace(PostUser, "'", "''") & "')")
GOVVal = Val(CBAL2)
PERFCHECK3
Else
HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & TT11 & "','" & TanG & "','" & Muka & "','" & Val(Tan5) & "','0','" & Replace(PostUser, "'", "''") & "')")
End If

HConn.Execute ("insert into tblBookLedgerTemp values('" & Year(Date) & "','" & Date & "','" & C24 & "','" & TanG & "','" & Muka & "','0','" & Val(Tan5) & "','" & Replace(PostUser, "'", "''") & "')")


HConn.Execute ("insert into tblClientInterest values ('" & Replace(rst!ClientName, "'", "''") & "','" & rst!AccID & "','" & Date & "','" & Joe2 & "','" & Joe4 & "','" & Year(Date) & "','" & Replace(PostUser, "'", "''") & "','" & Muka & "','')")


Dante:
rst.MoveNext

Loop

HConn.Execute ("insert into tblClientInterestdone values('" & Month(Date) & "','" & Year(Date) & "','" & Replace(PostUser, "'", "''") & "','" & DataCombo1.Text & "')")
End If


'SECOND SET

Set rst15 = HConn.Execute("Select distinct AccID as AccID from tblBookLedgerTemp order by AccID ")
If rst.BOF Then
Else
Do Until rst15.EOF

Set rst16 = HConn.Execute("Select sum(credit) - sum(debit) as Bal from tblBookLedgerTemp where AccID = '" & rst15!AccID & "'")
AB1 = Round(IIf(IsNull(rst16!Bal), 0, rst16!Bal), 2)

Set rst2 = HConn.Execute("Select * from tblBookLedgerTemp Where AccID = '" & rst15!AccID & "' ")
Shuss = Mudas '& " " & DataCombo1.Text

If Val(AB1) > 0 Then
   HConn.Execute ("Insert into tblBookLedger values ('" & rst2!gPeriod & "','" & rst2!PostDate & "','" & rst15!AccID & "','" & Replace(Shuss, "'", "''") & "','" & rst2!GREF & "','0','" & Val(AB1) & "','" & rst2!Puser & "')")
ElseIf Val(AB1) < 0 Then
   HConn.Execute ("Insert into tblBookLedger values ('" & rst2!gPeriod & "','" & rst2!PostDate & "','" & rst15!AccID & "','" & Replace(Shuss, "'", "''") & "','" & rst2!GREF & "','" & Val(AB1) * -1 & "','0','" & rst2!Puser & "')")
Else
End If

rst15.MoveNext
Loop
End If


MsgBox " The Interest Have Been Completed !", vbInformation

Answer 1

You only asked for suggestions how to make this code faster. However, after discussing what might be the issue with performance here, I will provide some comments regarding general maintainability issues.

Performance

Generally, what you are doing in the procedure on the client side would much better be handled on the database server side as a stored procedure. The general problem is that you are performing several network roundtrips for every record in the record set rst. These context switches are comparatively expensive, in particular should the database not reside on the same machine the code is running on, which would be a common setup.

This is one reason why you should try to do everything needed in one query.

A second reason to do so is that a database is much better optimized for matching data than your loop is. Looping over one table and then searching for the matching value in another table has a performance of \$O(NM)\$, which is not what you want for large tables. The merge and hash joins, databases use perform much better. A merge join would take \$O(N log(N) + M log(M))\$ under the worst possible conditions, e.g. with no indices and no ordering.

This brings up a point regarding the performance I cannot really judge because no details have been provided in the question, indices. These can make a vast difference in query performance. Should there not be an index on the AccId in the tables used in the loop, adding one should increase the performance. However, I guess there is one because I very much hope that there is a unique key constraint on the AccId in those tables. Otherwise, assumptions of the code would be violated.

Let me show what I mean with doing everything needed in one query by providing a query roughly returning everything you use from the part of the loop before inserting.

SELECT 
    CI.AccId,
    CI.CustBal1,
    C.Bal101,
    A.ACCTCODE
FROM tblCInterest AS CI
    INNER JOIN tblClients AS C
        ON CI.AccId = C.AccId 
            AND C.Blocked <> 'YES'
            AND C.Bal101 >= 0
    INNER JOIN tblSerialNo2 AS S
        ON C.Bal101 >= S.flopno
            AND (srefno = 0 
                OR srefno <= (SELECT COUNT(*) 
                              FROM tblSales AS Sa
                              WHERE Sa.AccId = C.AccId
                                  AND Wmonth = Joe4
                                  AND Wyear = Joe5)
    INNER JOIN tblAcctClasses AS A
        ON C.CCode = A.BCode 
            AND B.Acct = Datacombo1.Text
    LEFT JOIN tblClientInterest AS ClI
        ON CI.AccID = ClI.AccNo
            AND ClI.period = Month(Date)
            AND ClI.IntYear = Year(Date)
WHERE ClI.AccNo IS NULL
    AND CI.AccID LIKE 'GOOD%'
    AND Round(CI.CustBal1*Trim(Text1.Text)/12, 2) >= 0

Please note that I ignored any potentially necessary casts since I have no idea how the variables and columns are defined.

The second loop an also be condensed. This time the first two queries can be performed at once using a GROUP BY clause on the AccId. If you are running a sufficiently new version of SQL Server, you could do all three at once using an aggregate window functions. Otherwise, you can simply join the table again to the result of the GROUP BY query.

(For some reason, you seem to ever only use the first row from the last query, which is a bit odd.)

Apart from doing as much in one query as possible, it is also prudent not to return things you do not really need. This does not only save time on the database side, but also reduces the bandwidth you need to fetch the data. In this regard, it is wasteful to simply return all columns using *. Apart from the small performance gain, explicitly stating the columns also allows you to immediately get an error that a column you want is not there from the database instead of getting some error when you finally try to access it.

One other thing in this regard I found quite often in the code is that you return all columns from all matching rows, just to validate that something is there. You might just select a constant value NULL or 1, which would spare the database fetching anything from a table. Moreover, you could just fetch the first row or use an EXISTS subquery.

Security

At several places external text gets incorporated into your query strings. I hope that this text is either not entered by users or extensively validated. To understand why this is important, just google SQL injection attack or see http://bobby-tables.com/ for a quick reference.

Maintainability

Reading your code, I found several points where the maintainability of your code might be improved.

The very first thing is that the indentation of your code is rather inconsistent. Proper indentation can help very much making code more readable. To fix the indentation, you might want to visit the online VBA/VB6 indenter. (Full disclosure - I am involved in the project this website belongs to.)

Next come the variable names. I hope that the names are not the real names of the variables but dummies used here to protect some information. Generally, names should be descriptive. That also means that short names are usually a bad choice. With good names, reading code can be very close to reading a description of what the code is doing. The next maintainer of the code or you in a year will thank you for that.

Apart from the non-descriptive names, the code is very much harder to understand because you never declare the variables you use. For type safety reasons alone, declaring variables is already a good idea.

Another peculiarity in your code is that you use a lot of If Then Else constructs where either the Then block or the Else block is empty. In case of an empty Else block, you can simply not write Else with the same effect, but expressed in a clearer way. In case the Then block is empty, it would be a good idea to invert the condition. The you end up with Then and Else exchanged. Since the Else block is empty then, you can just remove it.

Another thing I observed is that the code uses GoTo commands outside of error handling statements. This is rather confusing and can always be handles without resorting to GoTo. In particular the first use of it could be converted to a Do While loop.

Finally, it struck me as odd that the AccId encodes the type of account. This would certainly better be stored in a different (indexed (joined with the AccId)) column. However, this is certainly nothing that could be changed easily after the database went into production.

Edit:

In addition to what I mentioned already, it might be a good idea to wrap the entire updating and inserting part of your procedure into a transaction. Otherwise, you risk that only part of it gets persisted to the database in case of a failure, be it due to an error or external reasons like a power outage.

You should actually wrap everything into a transaction with a suitable locking approach on the database to ensure that no concurrent process can change the customer balance while you are verifying and updating it.