Consulting

Results 1 to 7 of 7

Thread: Crashing UDF - debug tips needed

  1. #1
    VBAX Mentor
    Joined
    Aug 2012
    Posts
    367
    Location

    Crashing UDF - debug tips needed

    Hi all,
    I'm working on a user defined function that selects and sums three months worth of data from an annual data entry spreadsheet

    I have ID'd the data range of interest, but the function crashes when I attempt to interact with this range. one offending example is
    Set sumRange = mySheet.Range(Cells(lRow, lCol1), Cells(lRow, lCol2))
    processing this line causes the vbe to cease code execution entirely
    my row and column variables do have valid data and I'm getting confabulated over why this is happening.

    can anyone spot an error in my code, give any suggestions on how I might be able to trap the error or advise if there are any special rules with UDF's that I am breaking?

    the complete module is:
    Public Function SumResult(sOrg As String, sName As String) As Variant
    ' ----------------------------------------------------------------
    ' Purpose: sum quarter totals for results columns
    ' Parameter sOrg (String): ID Org data (start row)
    ' Parameter sName (String): ID row data to sum
    ' Return Type: Variant
    ' ----------------------------------------------------------------
    
    Dim myFinMonth As cls_FinMonth
    Dim lMonthFrom As Long
    Dim lMonthTo As Long
    Dim myResult As Variant
    Dim lRow As Long
    Dim lastRow As Long
    Dim mySheet As Worksheet
    Dim sumRange As Range
    
    Dim lCol1 As Long
    Dim lCol2 As Long
    
    Set mySheet = ThisWorkbook.Worksheets("2018-19")
    Set myFinMonth = New cls_FinMonth                       'initialise class
    
        myFinMonth.FinMonthName = ThisWorkbook.Worksheets("Admin").Range("nrMonthName")
        lastRow = mySheet.Cells.Find("*", SearchOrder:=xlByRows, SearchDirection:=xlPrevious).Row
    
        'ID month range
        lMonthTo = myFinMonth.FinMonthNum
        lMonthFrom = myFinMonth.FinQuarter * 3 - 2
        
        'ID data Row
        lRow = GetRow(sOrg, sName)
        
        'sum range intersect
        lCol1 = lMonthFrom + 3
        lCol2 = lMonthTo + 3
        Set sumRange = mySheet.Range(Cells(lRow, lCol1), Cells(lRow, lCol2))
        'myResult = Application.WorksheetFunction.Sum(sumRange)
        
        If myResult = 0 Then
            SumResult = "Code Error"
        Else
            SumResult = myResult
        End If
        
    End Function
    Remember: it is the second mouse that gets the cheese.....

  2. #2
    VBAX Master Aflatoon's Avatar
    Joined
    Sep 2009
    Location
    UK
    Posts
    1,720
    Location
    This is a common mistake. You need to qualify the Range and Cells properties with the same sheet:

    Set sumRange = mySheet.Range(mySheet.Cells(lRow, lCol1), mySheet.Cells(lRow, lCol2))
    On a separate note, this is bad design for a UDF, because you have to make it volatile to work properly due to the indirect references to other cells. All necessary ranges should be passed as direct arguments to the UDF ideally.
    Be as you wish to seem

  3. #3
    VBAX Sage
    Joined
    Apr 2007
    Location
    United States
    Posts
    8,728
    Location
    In addition to Aflatoon's comments, I'd try to avoid using a class inside a UDF, most for performance

    What is GetRow() ?


    An example workbook with a description of the desired results would help
    ---------------------------------------------------------------------------------------------------------------------

    Paul


    Remember: Tell us WHAT you want to do, not HOW you think you want to do it

    1. Use [CODE] ....[/CODE ] Tags for readability
    [CODE]PasteYourCodeHere[/CODE ] -- (or paste your code, select it, click [#] button)
    2. Upload an example
    Go Advanced / Attachments - Manage Attachments / Add Files / Select Files / Select the file(s) / Upload Files / Done
    3. Mark the thread as [Solved] when you have an answer
    Thread Tools (on the top right corner, above the first message)
    4. Read the Forum FAQ, especially the part about cross-posting in other forums
    http://www.vbaexpress.com/forum/faq...._new_faq_item3

  4. #4
    VBAX Mentor
    Joined
    Aug 2012
    Posts
    367
    Location
    thanks all
    this makes sense.

    Getrow is a function that takes the two user input fields to determine the correct row to sum, and returns this row number

    I take your point re the class inside a udf - this will cause unnecessary replication, and it is probably better to run this once and hold the value as a module level variable. I can maintain this via a worksheet change event. thanks for the tip.

    Werafa
    Remember: it is the second mouse that gets the cheese.....

  5. #5
    VBAX Sage
    Joined
    Apr 2007
    Location
    United States
    Posts
    8,728
    Location
    Ok, but it still seems unnecessarily complex for a UDF that might be used many times on a worksheet

    Dumb question -- ARE you planning to use this on a worksheet (which is the definiton of a UDF) or as a function called by other macros?
    ---------------------------------------------------------------------------------------------------------------------

    Paul


    Remember: Tell us WHAT you want to do, not HOW you think you want to do it

    1. Use [CODE] ....[/CODE ] Tags for readability
    [CODE]PasteYourCodeHere[/CODE ] -- (or paste your code, select it, click [#] button)
    2. Upload an example
    Go Advanced / Attachments - Manage Attachments / Add Files / Select Files / Select the file(s) / Upload Files / Done
    3. Mark the thread as [Solved] when you have an answer
    Thread Tools (on the top right corner, above the first message)
    4. Read the Forum FAQ, especially the part about cross-posting in other forums
    http://www.vbaexpress.com/forum/faq...._new_faq_item3

  6. #6
    VBAX Mentor
    Joined
    Aug 2012
    Posts
    367
    Location
    Hi Paul,
    yes I am.

    I could easily avoid using the class module, but I recently decided to learn to use them, and this is my second implementation.
    there isn't much code in the class - it is an object that synchronises month name, month number and financial month number.

    The udf is to be used in a report - so there is little to no user interaction. I will add one more variable so I can point it to a three-dimensional table structure and return one of three answers.

    so yes, it is probably not the most efficient code but I will be satisfied if it does the required task. I am also happy that I am beginning to understand how classes work and how powerful they can be.

    thanks again for your interest
    Werafa
    Remember: it is the second mouse that gets the cheese.....

  7. #7
    Whatever you do, take Aflatoon's advice on passing all ranges as arguments very seriously. In short: Any range of cells the UDF needs should be passed to the UDF as an argument. Using a worksheet change event to handle how a UDF works is a very unusual way of doing things.
    Regards,

    Jan Karel Pieterse
    Excel MVP jkp-ads.com

Posting Permissions

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