Help in simplifying code

**RaquelAR** · 11-16-2013, 09:02 PM

Hi!

I don't how to make some code of my macro more compact.

Here's and example of the code (it's too long to paste it all here):



Private Sub ParaUnidades()

With Application.WorksheetFunction

If ComboBox34.Value = "Unidades" And TextBox38.Value <> "" Then
    If DistribsOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Distribuitors", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If FarmaciasIOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Pharmacy Distribuitors", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If WHDOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Sales Force Wholesalers", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If DepDentOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "DEPOSITOS DENTALES", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If FarmaciasDOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Drugstores", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If HiperOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Hypermarkets", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If SuperOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Supermarkets", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If ClubsOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Clubs", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If TiendasDepOB.Value = True Then
    TextBox38.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Department Stores", Sheet9.Range("H:H"), ComboBox13.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
End If

If ComboBox34.Value = "Unidades" And TextBox39.Value <> "" Then
    If DistribsOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Distribuitors", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If FarmaciasIOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Pharmacy Distribuitors", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If WHDOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Sales Force Wholesalers", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If DepDentOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "DEPOSITOS DENTALES", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If FarmaciasDOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Drugstores", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If HiperOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Hypermarkets", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If SuperOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Supermarkets", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If ClubsOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Clubs", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
    If TiendasDepOB.Value = True Then
    TextBox39.Value = Format(.Round((.SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")) / .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("E:E"))) - (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), "Department Stores", Sheet9.Range("H:H"), ComboBox14.Value) / 100) * .SumIf(Sheet2.Range("A:A"), ComboBox14.Value, Sheet2.Range("D:D")), 2), "Currency")
    End If
End If

End With

There are a lot more comboboxes like those.

Thank you!

**Greg M** · 11-16-2013, 10:23 PM

Hi there,

I haven't checked your code in detail, but it would seem that your routines are crying out for a few User-Defined Functions!

I've written a function (mdMyCalculatedValue) which I think will work in conjunction with your "ParaUnidades" routine - obviously you should change the function name to whatever is appropriate (e.g. mdMonthlySales - the first "m" is normally used to define a Module-Level function and the "d" is normally used to indicate that the function returns a Double value)

The advantage of using User-Defined functions in this way is that any future changes are confined to a single routine.

If you follow the approach used in the code shown below you should be able to produce the equivalent shortening of the other routines in your application:




Private Sub ParaUnidades()

    If ComboBox34.Value = "Unidades" And TextBox38.Value <> "" Then

        With TextBox38

            If DistribsOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Distribuitors")

            ElseIf FarmaciasIOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Pharmacy Distribuitors")

            ElseIf WHDOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Sales Force Wholesalers")

            ElseIf DepDentOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="DEPOSITOS DENTALES")

            ElseIf FarmaciasDOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Drugstores")

            ElseIf HiperOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Hypermarkets")

            ElseIf SuperOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Supermarkets")

            ElseIf ClubsOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Clubs")

            ElseIf TiendasDepOB.Value = True Then
                   .Value = mdMyCalculatedValue(sOutlet:="Department Stores")

            End If

        End With

    End If

End Sub


Private Function mdMyCalculatedValue(sOutlet As String) As Double

    With WorksheetFunction

        mdMyCalculatedValue = Format(.Round(( _
            .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("D:D")) / _
            .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, Sheet2.Range("E:E"))) - _
 _
           (.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), sOutlet, _
                                         Sheet9.Range("H:H"), ComboBox13.Value) / 100) * _
 _
            .SumIf(Sheet2.Range("A:A"), ComboBox13.Value, _
                   Sheet2.Range("D:D")), 2), "Currency")

    End With

End Function

I hope you find the above helpful - please let me know how you get on.

Regards,

Greg M

**RaquelAR** · 11-16-2013, 11:12 PM

That's a lot better, Greg!

Thank you very much ^^ I'll be using it in other parts of the code.

**RaquelAR** · 11-17-2013, 01:49 AM

Unfortunately.... I tried to make it better and now I'm facing a problem ^^"

So I have this which is working fine:

Private Sub ParaCajas()

For i = 1 To 7
If Me.Controls("SKU" & i).Value <> "" Then
        With Me.Controls("Price" & i)
            If DistribsOB.Value = True Then
            .Value = amntcases(sOutlet = "Distribuitors")
            ElseIf FarmaciasIOB.Value = True Then
            .Value = amntcases(sOutlet = "Pharmacy Distribuitors")
            ElseIf WHDOB.Value = True Then
            .Value = amntcases(sOutlet = "Sales Force Wholesalers")
            ElseIf DepDentOB.Value = True Then
            .Value = amntcases(sOutlet = "DEPOSITOS DENTALES")
            ElseIf FarmaciasDOB.Value = True Then
            .Value = amntcases(sOutlet = "Drugstores")
            ElseIf HiperOB.Value = True Then
            .Value = amntcases(sOutlet = "Hypermarkets")
            ElseIf SuperOB.Value = True Then
            .Value = amntcases(sOutlet = "Supermarkets")
            ElseIf ClubsOB.Value = True Then
            .Value = amntcases(sOutlet = "Clubs")
            ElseIf TiendasDepOB.Value = True Then
            .Value = amntcases(sOutlet = "Department Stores")
            End If
        End With
Next
End Sub

But then I have this:

Private Function amntcases(sOutlet As String) As Double

    i = 1
    With Me.Controls("SKU" & i)
            amntcases = Format(WorksheetFunction.Round(WorksheetFunction.SumIf(Sheet2.Range("A:A"), .Value, Sheet2.Range("D:D")) - (WorksheetFunction.SumIfs(Sheet9.Range("I:I"), Sheet9.Range("E:E"), sOutlet, Sheet9.Range("H:H"), .Value) / 100) * WorksheetFunction.SumIf(Sheet2.Range("A:A"), .Value, Sheet2.Range("D:D")), 2), "Currency")
    End With
    
End Function

which is working fine as well BUT only because it has i = 1 :P

What I'm trying to do is:

use the same counter in "ParaUnidades" for "amntcases".

**Greg M** · 11-17-2013, 06:39 AM

Hi again,

Try this:



Private Sub ParaCajas()

    Dim i As Integer

    For i = 1 To 7

        If Me.Controls("SKU" & i).Value <> "" Then

            With Me.Controls("Price" & i)

                If DistribsOB.Value = True Then
                        .Value = amntcases(sOutlet:="Distribuitors", i:=i)

                ElseIf FarmaciasIOB.Value = True Then
                        .Value = amntcases(sOutlet:="Pharmacy Distribuitors", i:=i)

                ElseIf WHDOB.Value = True Then
                        .Value = amntcases(sOutlet:="Sales Force Wholesalers", i:=i)

                ElseIf DepDentOB.Value = True Then
                        .Value = amntcases(sOutlet:="DEPOSITOS DENTALES", i:=i)

                ElseIf FarmaciasDOB.Value = True Then
                        .Value = amntcases(sOutlet:="Drugstores", i:=i)

                ElseIf HiperOB.Value = True Then
                        .Value = amntcases(sOutlet:="Hypermarkets", i:=i)

                ElseIf SuperOB.Value = True Then
                        .Value = amntcases(sOutlet:="Supermarkets", i:=i)

                ElseIf ClubsOB.Value = True Then
                        .Value = amntcases(sOutlet:="Clubs", i:=i)

                ElseIf TiendasDepOB.Value = True Then
                        .Value = amntcases(sOutlet:="Department Stores", i:=i)

                End If

            End With

        End If

    Next

End Sub


Private Function amntcases(sOutlet As String, i As Integer) As Double

    With Me.Controls("SKU" & i)

        amntcases = _
            Format( _
                WorksheetFunction.Round( _
 _
                    WorksheetFunction.SumIf(Sheet2.Range("A:A"), .Value, _
                                            Sheet2.Range("D:D")) - _
 _
                   (WorksheetFunction.SumIfs(Sheet9.Range("I:I"), _
                                             Sheet9.Range("E:E"), sOutlet, _
                                             Sheet9.Range("H:H"), .Value) / 100) * _
 _
                    WorksheetFunction.SumIf(Sheet2.Range("A:A"), .Value, _
                                            Sheet2.Range("D:D")), 2), _
 _
                   "Currency")

    End With


End Function

Please let me know how this works for you.

Regards,

Greg M

**RaquelAR** · 11-17-2013, 10:53 AM

Greg M... YOU'RE A LIFESAVER.

Thank you very much ^^ +rep for you again.

**Greg M** · 11-17-2013, 02:04 PM

Hi again Raquel,

You're very welcome! Many thanks for your feedback and your kind words - I'm delighted that I was able to help.

For future reference, you can make your code a LOT more user-friendly by breaking it down into bite-sized chunks as I've tried to illustrate in the following:




Private Function amntcases(sOutlet As String, i As Integer) As Double

'   Assuming that the value in the SKU Control is assigned to a variable of type Double
    Dim dSkuValue   As Double

'   Dim iSkuValue   As Integer                  '   Use as appropriate
'   Dim lSkuValue   As Long                     '   Use as appropriate
'   Dim dteSkuValue As Date                     '   Use as appropriate

    Dim rLiquids    As Range
    Dim rSolids     As Range
    Dim rGases      As Range
    Dim rDates      As Range
    Dim rTimes      As Range

    With Sheet9
        Set rLiquids = .Range("I:I")
        Set rSolids = .Range("E:E")
        Set rGases = .Range("H:H")
    End With

    With Sheet2
        Set rDates = .Range("A:A")
        Set rTimes = .Range("D:D")
    End With

    dSkuValue = CDbl(Me.Controls("SKU" & i))
'   iSkuValue = CInt(Me.Controls("SKU" & i))    '   Use as appropriate
'   lSkuValue = CLng(Me.Controls("SKU" & i))    '   Use as appropriate
'   dteSkuValue = CDate(Me.Controls("SKU" & i)) '   Use as appropriate

    With WorksheetFunction

        amntcases = _
            (.SumIf(rDates, dSkuValue, rTimes) - _
            (.SumIfs(rLiquids, rSolids, sOutlet, rGases, dSkuValue) / 100) * _
             .SumIf(rDates, .Value, rTimes))

    End With

    amntcases = Round(amntcases, 2)

    amntcases = Format(amntcases, "Currency")

End Function

I've assigned names such as "Solids", "Liquids", "Gases" etc. to the various ranges because obviously I have no idea of what sort of data your worksheets contain

Please let me know if you need help with the above or with anything else.

Best regards,

Greg M

Help in simplifying code

LinkBack

Thread Tools

Rate This Thread

Display

Hybrid View

Help in simplifying code

Re: Help in simplifying code

Re: Help in simplifying code

Re: Help in simplifying code

Re: Help in simplifying code

Re: Help in simplifying code

Re: Help in simplifying code

Thread Information

Users Browsing this Thread

Similar Threads

Need help simplifying a code Please

Simplifying the Code

Simplifying the code

Simplifying code

Simplifying VB code

Bookmarks

Bookmarks

Posting Permissions