Procedure Too Large! Suggestions Please

A

Adam McReynolds

Registered User.
Local time
Yesterday, 21:36
Joined
Aug 6, 2012
Messages
129
I have this code that is giving me a compile error for procedure too large. Any suggestions on how I can keep the functionality but reduce the code? I already changed many If statements to case statements under the suggestion that they are more efficient. Here is just one set of code of 10 in the procedure. The entire code is too large to post:
Code: 
'###############################
'FIELD 1
'###############################

'Main condition to insert and update
Select Case Me.txt_prikey1
Case Is = Not IsNull(Me.txt_prikey1)
    '*********************************
    'Field 1 If Transfer and Hold(If ID in field then Insert to two Tables and Update one)
    '*********************************
    Select Case Me.txt_to_techid1
    Case Is = Not IsNull(Me.txt_prikey1)
        'Case for how to save status and transfer tech to release hold back later
        Select Case Me.txt_current_status1
            
            Case Is = "In Rework"
                'SQL INSERT - Transfer Log Table(TBL_TRANSFER_LOG)
                DoCmd.SetWarnings False
                strSQLInsertTransfer = "INSERT INTO TBL_TRANSFER_LOG ([prikey], [FromTechID], [FromTechName], " & _
                " [ToTechID], [ToTechName], [OriginalTimeIn], [TransferTime], " & _
                "[TransferRepairStatus], [TransferType], [ModuleType] , [ModulePartNumber] , [ModuleManufacturer], [ModuleBarcode], [ModuleSerialNumber] , [ModuleRMA])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
                Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & Nz(Me.txt_to_techid1) & "', '" & _
                Nz(Me.txt_to_tech_name1) & "', '" & Nz(Me.txt_original_time_in1) & "', '" & _
                Nz(Now) & "', '" & Nz(Me.txt_current_status1) & "','Hold Transfer', '" & Nz(Me.txt_mod_type1) & "', '" & Nz(Me.txt_part_num1) & "', '" & Nz(Me.txt_mfg1) & "', '" & Nz(Me.txt_bc1) & "', '" & _
                Nz(Me.txt_sn1) & "' , '" & Nz(Me.txt_rma1) & "');"
                Debug.Print strSQLInsertTransfer
                DoCmd.RunSQL strSQLInsertTransfer
                DoCmd.SetWarnings True
                
                'SQL INSERT - Hold Log Table(TBL_RF_HOLD_LOG)
                DoCmd.SetWarnings False
                strSQLInsertHold = "INSERT INTO TBL_RF_HOLD_LOG ([prikey], [Barcode], [SerialNumber], " & _
                " [EndUser], [HoldTechId], [HoldTechName], [RepairStatusTimeOfHold], " & _
                "[TimeOfHold], [HoldNotes], [Transfer] , [NewTechId] , [NewTechName])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
                Nz(Me.txt_bc1) & "', '" & Nz(Me.txt_sn1) & "', '" & Nz(Me.txt_end_user1) & "', '" & _
                Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & _
                Nz(Me.txt_current_status1) & "', '" & Nz(Now) & "','" & Nz(Me.txt_hold_notes1) & "', 'Yes', '" & Nz(Me.txt_to_techid1) & "','" & _
                Nz(Me.txt_to_tech_name1) & "');"
                Debug.Print strSQLInsertHold
                DoCmd.RunSQL strSQLInsertHold
                DoCmd.SetWarnings True
                
                'SQL UPDATE - Module Repairs table(affect RepairStatus and TechID for transfer. Stage in cycle depends on which tech to change)
                'This part of code is reason for case embedded into
                DoCmd.SetWarnings False
                SQLtextUpdateStatusHold = "update tbl_module_repairs set RepairStatus = 'On Hold' where prikey = " & Me.txt_prikey1.Value & ""
                SQLtextReworkTechNew = "update tbl_module_repairs set ReworkTech = '" & Me.txt_to_techid1 & "' where prikey = " & Me.txt_prikey1.Value & ""
                SQLtextReworkTimeInNew = "update tbl_module_repairs set ReworkTimeIn = Now where prikey = " & Me.txt_prikey1.Value & ""
                Debug.Print SQLtextUpdateStatusHold
                DoCmd.RunSQL SQLtextUpdateStatusHold
                DoCmd.RunSQL SQLtextReworkTechNew
                DoCmd.RunSQL SQLtextReworkTimeInNew
                DoCmd.SetWarnings True
                
            Case Is = "In Repair"
                'SQL INSERT - Transfer Log Table(TBL_TRANSFER_LOG)
                DoCmd.SetWarnings False
                strSQLInsertTransfer = "INSERT INTO TBL_TRANSFER_LOG ([prikey], [FromTechID], [FromTechName], " & _
                " [ToTechID], [ToTechName], [OriginalTimeIn], [TransferTime], " & _
                "[TransferRepairStatus], [TransferType], [ModuleType] , [ModulePartNumber] , [ModuleManufacturer], [ModuleBarcode], [ModuleSerialNumber] , [ModuleRMA])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
                Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & Nz(Me.txt_to_techid1) & "', '" & _
                Nz(Me.txt_to_tech_name1) & "', '" & Nz(Me.txt_original_time_in1) & "', '" & _
                Nz(Now) & "', '" & Nz(Me.txt_current_status1) & "','Hold Transfer', '" & Nz(Me.txt_mod_type1) & "', '" & Nz(Me.txt_part_num1) & "', '" & Nz(Me.txt_mfg1) & "', '" & Nz(Me.txt_bc1) & "', '" & _
                Nz(Me.txt_sn1) & "' , '" & Nz(Me.txt_rma1) & "');"
                Debug.Print strSQLInsertTransfer
                DoCmd.RunSQL strSQLInsertTransfer
                DoCmd.SetWarnings True
                
                'SQL INSERT - Hold Log Table(TBL_RF_HOLD_LOG)
                DoCmd.SetWarnings False
                strSQLInsertHold = "INSERT INTO TBL_RF_HOLD_LOG ([prikey], [Barcode], [SerialNumber], " & _
                " [EndUser], [HoldTechId], [HoldTechName], [RepairStatusTimeOfHold], " & _
                "[TimeOfHold], [HoldNotes], [Transfer] , [NewTechId] , [NewTechName])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
                Nz(Me.txt_bc1) & "', '" & Nz(Me.txt_sn1) & "', '" & Nz(Me.txt_end_user1) & "', '" & _
                Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & _
                Nz(Me.txt_current_status1) & "', '" & Nz(Now) & "','" & Nz(Me.txt_hold_notes1) & "', 'Yes', '" & Nz(Me.txt_to_techid1) & "','" & _
                Nz(Me.txt_to_tech_name1) & "');"
                Debug.Print strSQLInsertHold
                DoCmd.RunSQL strSQLInsertHold
                DoCmd.SetWarnings True
                
                'SQL UPDATE - Module Repairs table(affect RepairStatus and TechID for transfer. Stage in cycle depends on which tech to change)
                'This part of code is reason for case embedded into
                DoCmd.SetWarnings False
                SQLtextUpdateStatusHold = "update tbl_module_repairs set RepairStatus = 'On Hold' where prikey = " & Me.txt_prikey1.Value & ""
                SQLtextRepairTechNew = "update tbl_module_repairs set RepairTech = '" & Me.txt_to_techid1 & "' where prikey = " & Me.txt_prikey1.Value & ""
                SQLtextRepairTimeInNew = "update tbl_module_repairs set RepairTimeIn = Now where prikey = " & Me.txt_prikey1.Value & ""
                Debug.Print SQLtextUpdateStatusHold
                DoCmd.RunSQL SQLtextUpdateStatusHold
                DoCmd.RunSQL SQLtextRepairTechNew
                DoCmd.RunSQL SQLtextRepairTimeInNew
                DoCmd.SetWarnings True

        
            Case Is = "In QC"
                'SQL INSERT - Transfer Log Table(TBL_TRANSFER_LOG)
                DoCmd.SetWarnings False
                strSQLInsertTransfer = "INSERT INTO TBL_TRANSFER_LOG ([prikey], [FromTechID], [FromTechName], " & _
                " [ToTechID], [ToTechName], [OriginalTimeIn], [TransferTime], " & _
                "[TransferRepairStatus], [TransferType], [ModuleType] , [ModulePartNumber] , [ModuleManufacturer], [ModuleBarcode], [ModuleSerialNumber] , [ModuleRMA])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
                Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & Nz(Me.txt_to_techid1) & "', '" & _
                Nz(Me.txt_to_tech_name1) & "', '" & Nz(Me.txt_original_time_in1) & "', '" & _
                Nz(Now) & "', '" & Nz(Me.txt_current_status1) & "','Hold Transfer', '" & Nz(Me.txt_mod_type1) & "', '" & Nz(Me.txt_part_num1) & "', '" & Nz(Me.txt_mfg1) & "', '" & Nz(Me.txt_bc1) & "', '" & _
                Nz(Me.txt_sn1) & "' , '" & Nz(Me.txt_rma1) & "');"
                Debug.Print strSQLInsertTransfer
                DoCmd.RunSQL strSQLInsertTransfer
                DoCmd.SetWarnings True
                
                'SQL INSERT - Hold Log Table(TBL_RF_HOLD_LOG)
                DoCmd.SetWarnings False
                strSQLInsertHold = "INSERT INTO TBL_RF_HOLD_LOG ([prikey], [Barcode], [SerialNumber], " & _
                " [EndUser], [HoldTechId], [HoldTechName], [RepairStatusTimeOfHold], " & _
                "[TimeOfHold], [HoldNotes], [Transfer] , [NewTechId] , [NewTechName])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
                Nz(Me.txt_bc1) & "', '" & Nz(Me.txt_sn1) & "', '" & Nz(Me.txt_end_user1) & "', '" & _
                Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & _
                Nz(Me.txt_current_status1) & "', '" & Nz(Now) & "','" & Nz(Me.txt_hold_notes1) & "', 'Yes', '" & Nz(Me.txt_to_techid1) & "','" & _
                Nz(Me.txt_to_tech_name1) & "');"
                Debug.Print strSQLInsertHold
                DoCmd.RunSQL strSQLInsertHold
                DoCmd.SetWarnings True
                
                'SQL UPDATE - Module Repairs table(affect RepairStatus and TechID for transfer. Stage in cycle depends on which tech to change)
                'This part of code is reason for case embedded into
                DoCmd.SetWarnings False
                SQLtextUpdateStatusHold = "update tbl_module_repairs set RepairStatus = 'On Hold' where prikey = " & Me.txt_prikey1.Value & ""
                SQLtextQC_TechNew = "update tbl_module_repairs set QC_Tech = '" & Me.txt_to_techid1 & "' where prikey = " & Me.txt_prikey1.Value & ""
                SQLtextQC_TimeInNew = "update tbl_module_repairs set QC_TimeIn = Now where prikey = " & Me.txt_prikey1.Value & ""
                Debug.Print SQLtextUpdateStatusHold
                DoCmd.RunSQL SQLtextUpdateStatusHold
                DoCmd.RunSQL SQLtextQC_TechNew
                DoCmd.RunSQL SQLtextQC_TimeInNew
                DoCmd.SetWarnings True
            
            Case Else
                MsgBox "MAJOR ERROR!!! IF YOU ARE READING THIS YOU HAVE CAUSED A MAJOR ERROR. REPORT ASAP!", , "MAJOR ERROR!!!!!"
        
        End Select
    
    Case Else
    '*********************************
    'Field 1 Hold Only(No Transfer)(No ID in field Then Insert to one Table and Update one. No need for case as all should be same.)
    'Completed units restricted at barcode after update event
    '*********************************

        'SQL INSERT - Hold Log Table(TBL_RF_HOLD_LOG)
        DoCmd.SetWarnings False
        strSQLInsertHold = "INSERT INTO TBL_RF_HOLD_LOG ([prikey], [Barcode], [SerialNumber], " & _
        " [EndUser], [HoldTechId], [HoldTechName], [RepairStatusTimeOfHold], " & _
        "[TimeOfHold], [HoldNotes])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
        Nz(Me.txt_bc1) & "', '" & Nz(Me.txt_sn1) & "', '" & Nz(Me.txt_end_user1) & "', '" & _
        Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & _
        Nz(Me.txt_current_status1) & "', '" & Nz(Now) & "','" & Nz(Me.txt_hold_notes1) & "');"
        Debug.Print strSQLInsertHold
        DoCmd.RunSQL strSQLInsertHold
        DoCmd.SetWarnings True
        
        'SQL UPDATE - Module Repairs table(affect RepairStatus and TechID for transfer. Stage in cycle depends on which tech to change)
        'This part of code is reason for case embedded into
        DoCmd.SetWarnings False
        SQLtextUpdateStatusHold = "update tbl_module_repairs set RepairStatus = 'On Hold' where prikey = " & Me.txt_prikey1.Value & ""
        Debug.Print SQLtextUpdateStatusHold
        DoCmd.RunSQL SQLtextUpdateStatusHold
        DoCmd.SetWarnings True
                   
    End Select

    If Not IsNull(Me.txt_ber1) Then
        DoCmd.SetWarnings False
        SQLtextBER = "update tbl_module_repairs set incoming_disposition = '" & Me.txt_ber1 & "' where prikey = " & Me.txt_prikey1.Value & ""
        SQLtextBERCompleted = "update tbl_module_repairs set RepairStatus = 'Completed' where prikey = " & Me.txt_prikey1.Value & ""
        Debug.Print SQLtextBER
        DoCmd.RunSQL SQLtextBER
        DoCmd.RunSQL SQLtextBERCompleted
        DoCmd.SetWarnings True
    End If
    
    If Not IsNull(Me.txt_warr_status1) Then
        DoCmd.SetWarnings False
        SQLtextWarrStatus = "update tbl_module_repairs set WarrantyStatusFlag = '" & Me.txt_warr_status1 & "' where prikey = " & Me.txt_prikey1.Value & ""
        Debug.Print SQLtextWarrStatus
        DoCmd.RunSQL SQLtextWarrStatus
        DoCmd.SetWarnings True
    End If
    
    If Not IsNull(Me.txt_estimate_flag1) Then
        DoCmd.SetWarnings False
        SQLtextEstFlag = "update tbl_module_repairs set EstimateFlag = '" & Me.txt_estimate_flag1 & "' where prikey = " & Me.txt_prikey1.Value & ""
        Debug.Print SQLtextEstFlag
        DoCmd.RunSQL SQLtextEstFlag
        DoCmd.SetWarnings True
    End If


End Select
 
Adam McReynolds said:
..Any suggestions on how I can keep the functionality but reduce the code? ...
Click to expand...
Create a Sub/Procedure for the code under each Case Statement.

Code: 
'###############################
'FIELD 1
'###############################

'Main condition to insert and update
Select Case Me.txt_prikey1
Case Is = Not IsNull(Me.txt_prikey1)
    '*********************************
    'Field 1 If Transfer and Hold(If ID in field then Insert to two Tables and Update one)
    '*********************************
    Select Case Me.txt_to_techid1
    Case Is = Not IsNull(Me.txt_prikey1)
        'Case for how to save status and transfer tech to release hold back later
        Select Case Me.txt_current_status1
            Case Is = "In Rework"
              Call InRework
            Case Is = "In Repair"
              Call InRepair
            Case Is = "In QC"
              Call InQC
            ...
            ...
 
I have a similar problem. Wonder how it work out for the OP...
 
I have had longer procedures than 10 times that code that was posted, or at least I think I have. However, the advice to break it up into callable parts is good. Divide and conquer, particularly into small support routines in separate modules. Keeps things manageable and tightly organized.

I believe there might be a limit to the number of lines in a module, but that limit should be on the order of either 16383 or 32767 lines (don't remember which and too lazy to look it up). However, I don't see where having 9 more parts like what was posted could exceed those limits by itself, because that post wasn't nearly 1640 lines long. So it isn't strictly that procedure that is at fault - it has to be everything else in the same module around it PLUS the sample code.

I think the only other possible issues would be that the original poster might need to do a compact/repair or perhaps create a virgin database and import everything from the old database (on the theory that it has become corrupted in some way).
 
My best math professor use to dock us points for solving problems with more lines than necessary. You could get the right answer but do it less efficiently than a classmate who got the wrong answer but got there more efficiently and get less points than them.

You're code is very inefficient. You've 'copy programmed', that is, instead of programming efficiently, you copy and paste code all over the place. For example:

Code: 
                DoCmd.SetWarnings False
                strSQLInsertTransfer = "INSERT INTO TBL_TRANSFER_LOG ([prikey], [FromTechID], [FromTechName], " & _
                " [ToTechID], [ToTechName], [OriginalTimeIn], [TransferTime], " & _
                "[TransferRepairStatus], [TransferType], [ModuleType] , [ModulePartNumber] , [ModuleManufacturer], [ModuleBarcode], [ModuleSerialNumber] , [ModuleRMA])VALUES ('" & Nz(Me.txt_prikey1) & "','" & _
                Nz(Me.txt_techid_hold) & "', '" & Nz(Me.txt_from_tech_name1) & "', '" & Nz(Me.txt_to_techid1) & "', '" & _
                Nz(Me.txt_to_tech_name1) & "', '" & Nz(Me.txt_original_time_in1) & "', '" & _
                Nz(Now) & "', '" & Nz(Me.txt_current_status1) & "','Hold Transfer', '" & Nz(Me.txt_mod_type1) & "', '" & Nz(Me.txt_part_num1) & "', '" & Nz(Me.txt_mfg1) & "', '" & Nz(Me.txt_bc1) & "', '" & _
                Nz(Me.txt_sn1) & "' , '" & Nz(Me.txt_rma1) & "');"
                Debug.Print strSQLInsertTransfer
                DoCmd.RunSQL strSQLInsertTransfer
                DoCmd.SetWarnings True

You've used that exact block of code in 3 different cases. There is no reason for that. If you need a block of code in more than 2 places, you build a sub/function to run that code and you call that sub/function instead of copying and pasting code where you need it.

Also, back to my math analogy. Remember how you reduce algebraic expressions like this:

(39x + 26y + 13)

To this:

13*(3x + 2y + 1)

The same applies to code. If you do the same thing in both the IF and the ELSE, then there is no reason for that code to be inside the IF/ELSE:

IF (variable=True) THEN
sub()
variable2=99
ELSE
sub()
variable2=1
END IF

Since sub() is going to run in both spots, it shouldn't be inside the IF/ELSE. You're code has a lot of issues like that and the copying programming I discussed. You have a lot of room to reduce your code lines.
 
I made a post similar to this one however my question was more along the lines if it were more efficient to call subs/procedures then to simply have one long sub.

I received some great advice...I was able to make one sub and make other adjustments that reduced my overall number of lines from 1k+ to just over 800.

The the real performance gain was that I was able to eliminate the total number of variables and record sets...AND...added the Option Explicit line that significantly sped the performance.

This forum has been a Godsend for me...
 
As the Gent points out, there are times when code reduction is the right answer and as Plog points out, sometimes you reduce the total number of lines by simply looking at WHERE you need your calls. Usually it is a mix of methods that will optimize the code. Making some things into subroutine calls also helps since that also reduces your code footprint significantly.

All of these ideas should be in your toolkit for VBA management.
 
1) set warnings false once, at the top of the routine, and set warnings true at the bottom, and save 28 lines of code right there.
2) you are not required to assign SQL to a variable, and then run that SQL from the variable, for instance, you do this everywhere
Code: 
        SQLtextBER = "update tbl_module_repairs set incoming_disposition = '" & Me.txt_ber1 & "' where prikey = " & Me.txt_prikey1.Value & ""
        SQLtextBERCompleted = "update tbl_module_repairs set RepairStatus = 'Completed' where prikey = " & Me.txt_prikey1.Value & ""
        DoCmd.RunSQL SQLtextBER
        DoCmd.RunSQL SQLtextBERCompleted
...but you can just do
Code: 
        DoCmd.RunSQL "update tbl_module_repairs set incoming_disposition = '" & Me.txt_ber1 & "' where prikey = " & Me.txt_prikey1.Value & ""
        DoCmd.RunSQL "update tbl_module_repairs set RepairStatus = 'Completed' where prikey = " & Me.txt_prikey1.Value & ""
...which saves you 21 lines of code. Then delete your Debug.Print statements, 18 lines of code.

There's almost 70 lines of code, and presumably all those variables were declared somewhere, so that might be another 21 lines of code?
 
You must log in or register to reply here.

Similar threads

D
Solved Problem with dates in sql
Replies
7
Views
903
disgracept
D
V
Reuseable Where Condition
Replies
6
Views
1,446
plog
P
V
Dealing with Null value on SQL Statement
2
Replies
24
Views
3,019
theDBguy
theDBguy
gakiss2
Solved Payin' Cost for using Attachment - Round 2
Replies
13
Views
1,846
Gasman
Gasman
T
Remove For Each..Next
Replies
8
Views
941
tabitha
T

Users who are viewing this thread

Total: 1 (members: 0, guests: 1)
Back
Top Bottom