CODEWITHSUNDEEP
vba
amiros01
amiros01

Reputation: 3

function does not pass the value

please help me with this problem. I'm just starting to use VBA and after searching the forum was not able to find a solution. I have 1000 single digit numbers in my spreadsheet. 20 rows of 50 numbers in each. my program suppose to find the largest product. For some reason my final answer is 0. I've done some debugging and the program goes through all the loops and iterations as expected. I suspect that my function does not pass the value back to my main 

    Public Sub problem8()

Dim product, i, j, maxproduct As Long
maxproduct = product = 1

For i = 1 To 20
    For j = 1 To 50
        product = calcproduct(i, j)

        If product > maxproduct Then maxproduct = product
    Next j
Next i
Range("AY1").Value = maxproduct

End Sub

Function calcproduct(ByVal a As Long, ByVal b As Long) As Long

Dim i, j, count As Long
counter = calcproduct = 1
For i = a To 20
    For j = b To 50
        calcproduct = Cells(i, j).Value * calcproduct
        counter = counter + 1
        If counter = 13 Then Exit Function
    Next j
Next i

End Function

Upvotes: 0

Views: 38

Answers (2)

user4039065
user4039065

Reputation:

First off, this is the wrong way to express what you are trying to accomplish. 

maxproduct = product = 1

This says "maxproduct is equal to False". product was just declared so it is for all intents and purposes a zero and not equal to 1. You wanted to say,

maxproduct = 1
product = 1

If you really need to put those last two on a single line then use a colon like this,

maxproduct = 1: product = 1

The same holds true for the similar syntax in the function.

Now it is important to understand that VBA treats True as -1 and False as 0 (zero). Since you are essentially initializing maxproduct and product as zeroes, you can multiply anything you want by it and you will still end up with zero. Again, the same holds true for the way this similar variable assignment was handled in the function.

Here is my take on your project. 

Option Explicit

Public Const maxA As Long = 20
Public Const maxB As Long = 50
Public Const cntC As Long = 13

Public Sub problem8()
    Dim product As Long, i As Long, j As Long, maxproduct As Long
    maxproduct = 1: product = 1

    For i = 1 To maxA
        For j = 1 To maxB
            product = calcproduct(i, j)
            If product > maxproduct Then maxproduct = product
        Next j
    Next i
    Range("AY1").Value = maxproduct
End Sub

Function calcproduct(ByVal a As Long, ByVal b As Long) As Long
    Dim i As Long, j As Long, counter As Long
    counter = 1: calcproduct = 1
    For i = a To maxA
        For j = b To maxB
            calcproduct = Cells(i, j).Value * calcproduct
            counter = counter + 1
            If counter = cntC Then Exit Function
        Next j
    Next i
End Function

I moved the limits to public constants which make it easier to modify for different sized regions. In your function you declared a count then started using a var called counter so I changed that to suit. As mentioned in another post, the variable declaration needs to be specific or you end up with a bunch of variants and a few longs.

Upvotes: 0

Dan Donoghue
Dan Donoghue

Reputation: 6206

Change:

Dim product, i, j, maxproduct As Long

to:

Dim product as long, i as long, j as long, maxproduct As Long

and:

Dim i, j, count As Long

to:

Dim i as long, j as long, count As Long

I used to think putting it on the end applied it to everything on the row but it doesn't and I encountered a similar issue to you.

I might add though, the way you have done this (assigning i and j to new variables a and b then using i and j in the second routine) is extremely confusing and I would strongly recommend against it.

Upvotes: 1

Related Questions