Results 1 to 8 of 8
  1. #1
    2 Star Lounger
    Join Date
    May 2015
    Posts
    104
    Thanks
    15
    Thanked 0 Times in 0 Posts

    Need help with subroutines...

    Hi Guys,

    I been working on rewriting my code to simplify it. I made great headway, managed to lose over 50% of the code and still keep all the functionality of the excel document.
    I am just now getting into the realm of writing subroutines that would work from anywhere in the excel file and I have a little problem that I hope someone can clear up for me.

    I have a Work_update sub that calls in a subroutine and I can't figure out how to keep the value of LID once the subroutine starts...

    Code:
    Private Sub Worksheet_Change(ByVal Target As Range)
    
    
    If Target.Cells.Count > 2 Then Exit Sub                                     'ignore if multiple cells changed
                                                             '
    r = Target.Row                                                              'row number of cell that changed
    zRow = r Mod 24                                                             'trade template row; 24 rows per block
    zCol = Target.Column                                                        'column number of cell that changed
    
    
    Select Case zRow    
    '...........................................................................Case 4, 5, 6, 7, 8                                                          'change detected in Item qty & Item name rows..
    '...........................................................................
        
        Select Case zCol
        '.......................................................................
        Case [b1].Column, [i1].Column, [p1].Column, [w1].Column                 'item name deletion..
        '.......................................................................
        Application.EnableEvents = False                                        'turn events OFF during changes
        If Cells(r, zCol) = "" Then                                             'when first cell is deleted, clear all related cells..
            Range(Target.Offset(0, 1), Target.Offset(0, 4)) = ""
            Range(Target.Offset(6, 1), Target.Offset(6, 4)) = ""
            Range(Target.Offset(13, 1), Target.Offset(13, 4)) = ""
        End If
        Target.Offset(6) = Target.Value                                         'place first value in Cost
        Target.Offset(13) = Target.Value                                        'place first value in Return
        
        LID = Cells(r, zCol).Offset(2 - r)
        UpdateTotals                                                            'update Cost Total Cost, Total Return, Profit & Profit Margin..
    When in UpdateTotals, LID value is gone...

    Code:
    Sub UpdateTotals()
    
    
        Dim shtTrades As Worksheet
        Dim I As Long
        Set shtTrades = Worksheets("Trades")
        
        J = Chr(ActiveCell.Column + 64)             'B
        MsgBox LID 'it is nothing...
        LID = ActiveCell.Offset(2 - ActiveCell.Row) 'I would like to skip this so I can use it on other sheets as well...
    
    
    Select Case Chr(ActiveCell.Column + 64)
    
    
    Case "B"
    I = LID * 6 - 4
    Case "I"
    I = LID * 6 - 10
    Case "P"
    I = LID * 6 - 16
    Case "W"
    I = LID * 6 - 22
    End Select
    
    
    With shtTrades
        .Cells(I, J).Offset(13, 5) = WorksheetFunction.Sum(Range(.Cells(I, J).Offset(8, 5), .Cells(I, J).Offset(12, 5)))   'Cost Total
        If .Cells(I, J).Offset(13, 5) = 0 Then
            .Cells(I, J).Offset(13, 5) = ""
        End If
        .Cells(I, J).Offset(20, 5) = WorksheetFunction.Sum(Range(.Cells(I, J).Offset(15, 5), .Cells(I, J).Offset(19, 5)))  'Return Total
        If .Cells(I, J).Offset(20, 5) = 0 Then
            .Cells(I, J).Offset(20, 5) = ""
        End If
        .Cells(I, J).Offset(22, 5) = .Cells(I, J).Offset(20, 5) - .Cells(I, J).Offset(13, 5)                               'Total Profit
        If .Cells(I, J).Offset(22, 5) = 0 Then
            .Cells(I, J).Offset(22, 5) = ""
            .Cells(I, J).Offset(22) = ""
        Else
            .Cells(I, J).Offset(22) = .Cells(I, J).Offset(22, 5) / .Cells(I, J).Offset(13, 5)                              'Profit Margin
        End If
    End With
    
    
    End Sub
    I bet it is something simple, but no idea what...

    Ferenc

  2. #2
    WS Lounge VIP rory's Avatar
    Join Date
    Dec 2000
    Location
    Burwash, East Sussex, United Kingdom
    Posts
    6,280
    Thanks
    3
    Thanked 191 Times in 177 Posts
    You haven't declared LID anywhere that I can see and it is good practice to always declare all variables.

    The best option is to pass LID to the routine as an argument:
    Code:
    Sub UpdateTotals(LID as Variant)
    which you then call using either:
    Code:
     UpdateTotals LID
    or:
    Code:
    Call UpdateTotals(LID)
    Note: if you don't use Call, don't put parentheses around the parameters for the sub.
    Regards,
    Rory

    Microsoft MVP - Excel

  3. The Following User Says Thank You to rory For This Useful Post:

    Ferenc Nagy (2015-07-22)

  4. #3
    WS Lounge VIP
    Join Date
    Mar 2002
    Location
    Newcazzle, UK
    Posts
    2,832
    Thanks
    136
    Thanked 484 Times in 461 Posts
    Hi Ferenc

    In addition to Rory's note, and to further your goal of simplifying your code, you might consider using:
    Code:
    With shtTrades
    set zCell = .cells(i,j)
    costTotal = application.sum(zCell.offset(8,5,4,1))
    returnTotal = application.sum(zCell.offset(15,5,4,1))
    totalProfit = returnTotal - costTotal 
    
    if costTotal = 0 then costTotal = "" 
    zCell.Offset(13, 5) = costTotal 
    if returnTotal = 0 then returnTotal = "" 
    zCell.Offset(13, 5) = returnTotal
     
    if totalProfit = 0 then
     zCell.Offset(22, 0) = ""
     zCell.Offset(22, 5) = ""
    else
     zCell.Offset(22, 0) = totalProfit 
     zCell.Offset(22, 5) = returnTotal/costTotal 
    end if
    End With
    ..instead of your
    Code:
    With shtTrades
        .Cells(I, J).Offset(13, 5) = WorksheetFunction.Sum(Range(.Cells(I, J).Offset(8, 5), .Cells(I, J).Offset(12, 5)))   'Cost Total
        If .Cells(I, J).Offset(13, 5) = 0 Then
            .Cells(I, J).Offset(13, 5) = ""
        End If
        .Cells(I, J).Offset(20, 5) = WorksheetFunction.Sum(Range(.Cells(I, J).Offset(15, 5), .Cells(I, J).Offset(19, 5)))  'Return Total
        If .Cells(I, J).Offset(20, 5) = 0 Then
            .Cells(I, J).Offset(20, 5) = ""
        End If
        .Cells(I, J).Offset(22, 5) = .Cells(I, J).Offset(20, 5) - .Cells(I, J).Offset(13, 5)                               'Total Profit
        If .Cells(I, J).Offset(22, 5) = 0 Then
            .Cells(I, J).Offset(22, 5) = ""
            .Cells(I, J).Offset(22) = ""
        Else
            .Cells(I, J).Offset(22) = .Cells(I, J).Offset(22, 5) / .Cells(I, J).Offset(13, 5)                              'Profit Margin
        End If
    End With
    ..I haven't tested this!

    zeddy
    •Revenue Cycle Route Planner
    .

  5. #4
    WS Lounge VIP rory's Avatar
    Join Date
    Dec 2000
    Location
    Burwash, East Sussex, United Kingdom
    Posts
    6,280
    Thanks
    3
    Thanked 191 Times in 177 Posts
    Quote Originally Posted by zeddy View Post
    ..I haven't tested this!
    The Range.Offset method is not like the OFFSET worksheet function - it only takes two arguments. You need Resize as well:
    Code:
    costTotal = application.sum(zCell.offset(8,5).Resize(4,1))
    Regards,
    Rory

    Microsoft MVP - Excel

  6. The Following User Says Thank You to rory For This Useful Post:

    zeddy (2015-07-22)

  7. #5
    WS Lounge VIP
    Join Date
    Mar 2002
    Location
    Newcazzle, UK
    Posts
    2,832
    Thanks
    136
    Thanked 484 Times in 461 Posts
    Hi Rory

    ..thanks for clarifying that.
    It's not the sort of thing I would do in vba - as you know, it is better to do Excel calculations on the worksheet itself if you can, rather than in vba.

    zeddy
    •Internal Solar Sales Consultant

  8. #6
    2 Star Lounger
    Join Date
    May 2015
    Posts
    104
    Thanks
    15
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by zeddy View Post
    Hi Ferenc

    In addition to Rory's note, and to further your goal of simplifying your code, you might consider using:
    Code:
    With shtTrades
    set zCell = .cells(i,j)
    costTotal = application.sum(zCell.offset(8,5,4,1))
    returnTotal = application.sum(zCell.offset(15,5,4,1))
    totalProfit = returnTotal - costTotal 
    
    if costTotal = 0 then costTotal = "" 
    zCell.Offset(13, 5) = costTotal 
    if returnTotal = 0 then returnTotal = "" 
    zCell.Offset(13, 5) = returnTotal
     
    if totalProfit = 0 then
     zCell.Offset(22, 0) = ""
     zCell.Offset(22, 5) = ""
    else
     zCell.Offset(22, 0) = totalProfit 
     zCell.Offset(22, 5) = returnTotal/costTotal 
    end if
    End With
    ..instead of your
    Code:
    With shtTrades
        .Cells(I, J).Offset(13, 5) = WorksheetFunction.Sum(Range(.Cells(I, J).Offset(8, 5), .Cells(I, J).Offset(12, 5)))   'Cost Total
        If .Cells(I, J).Offset(13, 5) = 0 Then
            .Cells(I, J).Offset(13, 5) = ""
        End If
        .Cells(I, J).Offset(20, 5) = WorksheetFunction.Sum(Range(.Cells(I, J).Offset(15, 5), .Cells(I, J).Offset(19, 5)))  'Return Total
        If .Cells(I, J).Offset(20, 5) = 0 Then
            .Cells(I, J).Offset(20, 5) = ""
        End If
        .Cells(I, J).Offset(22, 5) = .Cells(I, J).Offset(20, 5) - .Cells(I, J).Offset(13, 5)                               'Total Profit
        If .Cells(I, J).Offset(22, 5) = 0 Then
            .Cells(I, J).Offset(22, 5) = ""
            .Cells(I, J).Offset(22) = ""
        Else
            .Cells(I, J).Offset(22) = .Cells(I, J).Offset(22, 5) / .Cells(I, J).Offset(13, 5)                              'Profit Margin
        End If
    End With
    ..I haven't tested this!

    zeddy
    •Revenue Cycle Route Planner
    .
    Hey Zeddy,

    Thank you for the continued support... I wanted to clarify my previous statement, what I described as simplifying my code, was actually to make it work if called from any sheet in the excel file, using the information that is available on any given page. This is the Location ID of the specific item...
    So in case anyone happens to be interested, this is how it looks the code now:

    Code:
    '****************************************************
    'Update Trade Totals..
    '****************************************************
    Sub TradeTotals(LID)
    
    
        Dim shtTrades As Worksheet
        Dim I As Long
        Dim J As Long
        Dim K As Long
        Dim L As Long
        Set shtTrades = Worksheets("Trades")
        J = 23
        zRow = LID * 6 - 22
    '.......................................................................
    With shtTrades                                                                  'Only looks at 3 possible locations
        For I = zRow To I + 18 Step 6
            If zRow < 0 Then                                                        'Handle the issue with Trade 1, 2, 3 & 4 => negative row number..
                zRow = zRow + 6
                GoTo Line1
            End If
            If .Cells(I, J).Value = LID Then
                '--------------------------------------------
                For K = 8 To 12 Step 1                                              'Cost Item Totals
                    If .Cells(I, J).Offset(K, 2) <> "" And .Cells(I, J).Offset(K, 4) <> "" Then
                        .Cells(I, J).Offset(K, 5) = .Cells(I, J).Offset(K, 2) * .Cells(I, J).Offset(K, 4)
                    End If
                Next K
                '--------------------------------------------
                For L = 15 To 19 Step 1                                             'Return Item Totals
                    If .Cells(I, J).Offset(L, 2) <> "" And .Cells(I, J).Offset(L, 4) <> "" Then
                        .Cells(I, J).Offset(L, 5) = .Cells(I, J).Offset(L, 2) * .Cells(I, J).Offset(L, 4)
                    End If
                Next L
                '--------------------------------------------
                Set zBlock = .Cells(I, J).Offset(8, 5).Resize(5)                    'Cost Total
                zSum1 = Application.Sum(zBlock)
                If zSum1 = 0 Then
                    .Cells(I, J).Offset(13, 5) = ""
                Else
                    .Cells(I, J).Offset(13, 5) = zSum1
                End If
                '--------------------------------------------
                Set zBlock = .Cells(I, J).Offset(15, 5).Resize(5)                   'Return Total
                zSum2 = Application.Sum(zBlock)
                If zSum2 = 0 Then
                    .Cells(I, J).Offset(20, 5) = ""
                Else
                    .Cells(I, J).Offset(20, 5) = zSum2
                End If
                '--------------------------------------------
                .Cells(I, J).Offset(22, 5) = zSum2 - zSum1                          'Total Profit
                If .Cells(I, J).Offset(22, 5) = 0 Then
                    .Cells(I, J).Offset(22, 5) = ""
                    .Cells(I, J).Offset(22) = ""
                Else
                    .Cells(I, J).Offset(22) = .Cells(I, J).Offset(22, 5) / zSum1    'Profit Margin
                End If
                '--------------------------------------------
                Exit For
            End If
    Line1:
            J = J - 7
        Next I
    End With
    '.......................................................................
    End Sub

  9. #7
    2 Star Lounger
    Join Date
    May 2015
    Posts
    104
    Thanks
    15
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by rory View Post
    You haven't declared LID anywhere that I can see and it is good practice to always declare all variables.

    The best option is to pass LID to the routine as an argument:
    Code:
    Sub UpdateTotals(LID as Variant)
    which you then call using either:
    Code:
     UpdateTotals LID
    or:
    Code:
    Call UpdateTotals(LID)
    Note: if you don't use Call, don't put parentheses around the parameters for the sub.
    Quick follow up:

    Why did you type UpdateTotals(LID as Variant) instead of UpdateTotals(LID)?

  10. #8
    Super Moderator RetiredGeek's Avatar
    Join Date
    Mar 2004
    Location
    Manning, South Carolina
    Posts
    9,436
    Thanks
    372
    Thanked 1,457 Times in 1,326 Posts
    Ferenc,

    LID would default to Variant but it is always better to be explicit then when someone is reading the code they will not make an incorrect assumption. HTH
    May the Forces of good computing be with you!

    RG

    PowerShell & VBA Rule!

    My Systems: Desktop Specs
    Laptop Specs

  11. The Following User Says Thank You to RetiredGeek For This Useful Post:

    Ferenc Nagy (2015-07-22)

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •