CODEWITHSUNDEEP
excelvbafunction
Julia
Julia

Reputation: 251

Call a function from sub in vba

I wrote this function, which is supposed to take the user defined cell from a chosen range and add or multiply its value by an user defined number. The output should be an array with one changed cell.

I was able to compile it; however, I cannot call it.

I tried to replicate what this and this but it failed.

Function Macro1(cellref As Range, row_number As Long, column_number As Long, x As Double, method As Integer) As Variant
'Number, which will be added or multiplied by chosen cell value in an array
'One for multiplication, two for addition
  If method = 1 Then
    Cells(row_number, column_number) = Cells(row_number, column_number) * x
  ElseIf method = 2 Then 
    Cells(row_number, column_number) = Cells(row_number, column_number) + x
  End If
  Result = Macro1
End Function

Sub try()
  Macro1(Range("A1:AX3").Select, 2, 2, 0.5, 1) = Result
End Sub

When running this code, I get the error message "Run-time error 424, object required". The line Macro1(Range("A1:AX3").Select, 2, 2, 0.5, 1) = Result gets red highlighted

Then I tried 

Function Macro1(cellref As Range, row_number As Long, column_number As Long, x As Double, method As Integer) As Variant
'Number, which will be added or multiplied by chosen cell value in an array
'One for multiplication, two for addition
  If method = 1 Then
    Cells(row_number, column_number) = Cells(row_number, column_number) * x
  ElseIf method = 2 Then 
    Cells(row_number, column_number) = Cells(row_number, column_number) + x
  End If
  Result = Macro1
End Function

Sub try()
  Macro1(Worksheets("Macro1").Range("A1:AX3"), 2, 2, 0.5, 1)
End Sub

But I got "Type mismatch Error 13" message. The line Sub try() is highlighted in yellow and the line below it is red

Upvotes: 0

Views: 1079

Answers (2)

Warcupine
Warcupine

Reputation: 4640

First, you don't need the cellref parameter you aren't using it.

as @nacorid and @Jnevill said result = Macro1 doesn't do anything, the code just skips it. If you want a return on your function it would be Macro1 = result. However, you don't assign anything to result so it still won't do anything. You would also need to create an assignment in sub try() to capture the return.

I am not sure what you mean by your expected output is an array with one changed cell, You are changing a cell but it isn't an array you are just directly changing the value of it with this line: Cells(row_number, column_number) = Cells(row_number, column_number) * x

Edit: Here is how you load up an array, modify and paste it somewhere else.

You will need to add some error handling to not get type mismatches.

You may also want to make it a little more dynamic with an offset number for the pasterange.

sub macro1()
    Dim rngarr() As Variant
    Dim divisor As Double
    Dim targetcol As Long
    Dim targetrow As Long
    Dim pasterange As Range

    rngarr = Range("A1:AX3").Value
    'Remember that your array starts at index 1
    'A range that starts at row 2 will still have an index 1
    targetcol = 2
    targetrow = 2

    divisor = 0.5

    rngarr(targetrow, targetcol) = rngarr(targetrow, targetcol) * divisor

    Set pasterange = Range("A5:AX7")

    pasterange = rngarr
end sub

Upvotes: 0

FreeMan
FreeMan

Reputation: 5687

I see several issues:

First, the line

Result = Macro1

In Macro1 is a recursive call to Macro1. i.e. Macro1 is trying to call itself.

You'll get an error because Macro1 requires parameters, the first of which is an Object (a Range is an Object), and you're not supplying an object parameter. As a matter of fact, you're not supplying any required parameters in this call.

Second, the code:

<functionName> = value

Is backwards. That line is attempting to set Macro1 (the <functionName> on the left-hand side of the assignment operator =) to the value of Result. This isn't possible (at least not in in VBA). I believe that what you want is:

result = Macro1(Worksheets("Macro1").Range("A1:AX3"), 2, 2, 0.5, 1)

Which will execute Macro1 with the specified parameters, then assign the returned value (a Variant as you've defined it) to the Variant variable (because you haven't Dimmed it) result.

Third, you've declared Macro1 to be a Function, but there's no need to - it's not returning a value, it's setting cells in the Range it's been passed. You'd be best served by changing this to a Sub, since that's the kind of work it's doing. I think you may have been intending to have it return a value with the line Result = Macro1, but as mentioned, that ain't gonna do it. VBA works by assigning the value to be returned to the Function name, not with a return function as some other languages do. If you do have some sort of result you want to return, you'd assign it with the code:

Macro1 = <the value to be returned>

However, I'm not sure what you'd actually be returning from this as a Function because it can work on a Range not an individual Cell

Fourth, I'm not entirely certain why you're accepting a whole Range as the parameter to Macro1, then specifying a single Cell within it. Passing in ...Range("A1:AX3"), then specifying I want column 2 and Row 2 of that range is way too much cognitive effort to sort out what cell you're after. Why not just pass in ...Range("B2") and be done with it? The benefits are

1) Macro1 works directly with the desired cell and the desired cell only

2) The caller specifies the desired cell and the desired cell only

3) You eliminate otherwise unnecessary parameters from the param list.

Fifth your Macro1 receives a Range parameter to work on, but never references it. You accept cellref As Range, but in the code body, you're working with Cells() and never once reference cellref. There are a couple of issues with this:

1) You have an unspecified Cells() which will automatically reference the ActiveSheet, whether that's your intent or not.

2) You're not at all working with whatever it is that you're passing in when you call Macro1 (which could be on any Worksheet in the Workbook).

You have done a great job of being very specific in your call to Macro1 by specifying Worksheets("Macro1").Range("A1:AX3") so that VBA knows exactly what cell you're after. However, if you happen to be looking at Sheet2 at the time of the call, Macro1 will be working with Worksheets("Sheet2").Range("B2") because at this point Activesheet = Worksheets("Sheet2") instead of Worksheets("Macro1").

Sixth you're doing no sanity checking on your input values. What happens if the value in the passed cell happens to be red instead of 17.256. What's the value of red * 2? Why, it's a Runtime error #13. Type mismatch. You need some sort of sanity checking or error handling to ensure you're working with numeric data.

In the end, I would suggest something like this with a pre-execution sanity check:

Function Macro1(cellref As Range, x As Double, method As Long) As Variant
'Number, which will be added or multiplied by chosen cell value in an array
'One for multiplication, two for addition

  Dim Result As Double

  If Not IsNumeric(cellref.Value) Then
    MsgBox ("The cell does not contain a numeric value")
    Exit Function
  End If

  If method = 1 Then
    Result = cellref.Value * x
  ElseIf method = 2 Then
    Result = cellref.Value + x
  End If
  Macro1 = Result

End Function

Sub try()
  With Worksheets("Sheet1")
    .Range("B2") = Macro1(.Range("B2"), 0.5, 1)
  End With
End Sub

Alternatively, you could go with error handling and do it like this:

Option Explicit

Function Macro1(cellref As Range, x As Double, method As Long) As Variant
'Number, which will be added or multiplied by chosen cell value in an array
'One for multiplication, two for addition

  On Error GoTo ErrorHandler

  Dim Result As Double

  If method = 1 Then
    Result = cellref.Value * x
  ElseIf method = 2 Then
    Result = cellref.Value + x
  End If

CleanExit:
  Macro1 = Result

ErrorHandler:
  MsgBox ("The cell does not contain a numeric value")
  Result = vbNull
  Resume CleanExit

End Function

Sub try()
  With Worksheets("Sheet1")
    .Range("B2") = Macro1(.Range("B2"), 0.5, 1)
  End With
End Sub

Notice the inclusion of Option Explicit that requires that you declare (Dim) all variables before you use them. This will help eliminate other potential errors where you misspell Variable1 as Varaible1 and VBA "helpfully" creates a whole new variable for you creating a very difficult bug to find.

Based on some of the other comments, I think that you are attempting to call this for many cells in a range. If that's the case, you can add some simple looping to the Try() procedure to run through all the cells that this needs to be applied to. If not, and you're somehow trying to apply this to one cell in a Range based on other values in that range, you'll need to modify some portion of it to do what you're after. I hope there's enough info in this post for you to figure out how to do that. If not, that would probably make for a great follow up question once you've fully grokked what's going on in changing this one cell.

As a side note, there is a great OSS VBE add-on called Rubberduck which will help you find and fix a lot of these types of errors. I'm a great fan of the tool, I use it daily, and I've contributed to the project as well.

Upvotes: 6

Related Questions