Optimize Redundant Code

jobrien4

Registered User.
Local time
Today, 12:53
Joined
Sep 12, 2011
Messages
51
I have a very large program that allows users to enter up to five quantities at once and the program returns a price for each quantity. I have the code structured so that the same code runs basically 5 times with one variable for each quantity. For example, take a look at the below code. I have several variables with the same name but a different number at the end.

Code:
Qty1
If Qty1 <> 0 Then
    
    Dim SlaggingTime1 As Single
    SlaggingTime1 = SlaggingTimePerPiece * Qty1
        
    Dim SlaggingAdminCharge1 As Single
    SlaggingAdminCharge1 = (SlaggingAdminChargeRate * Me.NetWeight1.Value) / 100
    
    If SlaggingAdminCharge1 < SlaggingAdminMinimum Then
        SlaggingAdminCharge1 = SlaggingAdminMinimum
    End If
        
    Dim SlaggingCharge1 As Single
    SlaggingCharge1 = (((SlaggingTime1 / 60) * SlaggingChargeRate) + SlaggingAdminCharge1) / Qty1
    
    Me.SlaggingCost1.Visible = True
    Me.SlaggingCost1.Value = SlaggingCharge1 * (1 - LaborDiscount)
Else
    Me.SlaggingCost1.Value = 0
    Me.SlaggingCost1.Visible = False
End If

' Qty2
If Qty2 <> 0 Then
    
    Dim SlaggingTime2 As Single
    SlaggingTime2 = SlaggingTimePerPiece * Qty2
        
    Dim SlaggingAdminCharge2 As Single
    SlaggingAdminCharge2 = (SlaggingAdminChargeRate * Me.NetWeight2.Value) / 100
    
    If SlaggingAdminCharge2 < SlaggingAdminMinimum Then
        SlaggingAdminCharge2 = SlaggingAdminMinimum
    End If
        
    Dim SlaggingCharge2 As Single
    SlaggingCharge2 = (((SlaggingTime2 / 60) * SlaggingChargeRate) + SlaggingAdminCharge2) / Qty2
    
    Me.SlaggingCost2.Visible = True
    Me.SlaggingCost2.Value = SlaggingCharge2 * (1 - LaborDiscount)
Else
    Me.SlaggingCost2.Value = 0
    Me.SlaggingCost2.Visible = False
End If

That block of code repeats three more times.

Is there a way for me to simplify this code and consolidate it but using either arrays or a do loop? Or could I create a function for this and feed it the Qty variable and get back the price?

This is a simple block of the code but there are other sections that are much more complex. I'm looking to simplify this program so it is easier to maintain and if possible, improve run-time efficiency. My programming skills are pretty rudimentary so I'm looking for best practice on handling this case.
 
Last edited:
1. Enumerations like Something1, Something2 ... SomethingN are most often indicative of structural problems in data. Show you data structure. Attach screenshot of Relations window with all tables shown and expanded fully.

2. What is "run-time efficiency?" That something takes 2 seconds instead of 1 is only important if you do this thing thousands of time, otherwise polishing it is a waste of manpower.

Update: But you ARE right in that it is better to have one piece of code to maintain than 5 :D
 
I've done a Google presentation on how to convert it HERE:-

PDF on Optimising Code


Wow that's great! Thanks

I was wondering if a function would be the way to go and I could just pass Qty1, Qty2, Qty3... plus all of the other needed variables and pull out the results. Where I got hung up was on the controls for each Quantity. When the price for a given quantity is calculated, it is displayed in that control as you pointed out.

I'm still not exactly clear from your slides how to handle that. I'll do some testing based on your slides and see if I can't figure it out. Thanks again
 
Last edited by a moderator:
Ok so I took some tips from your presentation. Also added a For statement that sets the control based on the iteration. This reduced my code from 175 lines to 85 lines just for this section. Thanks everyone

Code:
Dim Qty(4) As Integer
Qty(0) = Qty1
Qty(1) = Qty2
Qty(2) = Qty3
Qty(3) = Qty4
Qty(4) = Qty5

Dim strControlName As String

Dim n As Integer
n = 0

For n = 0 To 4
    strControlName = "SlaggingCost" & (n + 1)
    
    If Qty(n) <> 0 Then
        Form(strControlName).Visible = True
        Form(strControlName).Value = fCalculateSlaggingCharge(Qty(n), EffectiveNetWeight, SlaggingTimePerPiece, SlaggingChargeRate, SlaggingAdminChargeRate, SlaggingAdminMinimum, LaborDiscount)
    Else
        Form(strControlName).Visible = False
        Form(strControlName).Value = 0
    End If
Next n
 

Users who are viewing this thread

Back
Top Bottom