View Single Post
  #5   Report Post  
Posted to microsoft.public.excel.programming
joeu2004[_2_] joeu2004[_2_] is offline
external usenet poster
 
Posts: 829
Default Optional argument as Variant not acting like number

wrote:
Where I believe the code is failing is the line:
"If Mid(ProductCell.Offset(0, 1).Value, 4, 1) * 1 = ShiftNum * 1 Then"


Why do you only "believe" that? How is your function failing?

You can determine exactly where VBA is failing by:

1. Select the first executable line and press F9 to set a breakpoint.
Alternatively, add a Stop statement as the first executable statement.

2. Once stopped, repeatedly press F8 to single-step through the function.

Your original posting included a typo that I overlooked as, well, just a
posting typo. But perhaps it is real, to wit:

Function TotalCC(Week As String, Optional ShiftNum As Variant) As Integer
Set ProductRange = Sheets(WeekNum).Range("A1:A100")

If your function is failing with a #VALUE error, the reason might that
either Week should be WeekNum in the Function statement, or WeekNum should
be Week in the Set statement.


wrote:
The Mid statement (with or without the " * 1 "), never equals ShiftNum.
Even substituting ShiftNum with a number (1, 2, or 3) that I knew
'should' yield True in some cases, still failed to advance past that
statement.


Did you determine that by single-stepping through the function? If so....

I cannot tell you why Mid(ProductCell.Offset(0,1).Value,4,1) is not the
value that you expect. Try adding the following statement to be sure things
are what you expect:

Debug.Print Mid(ProductCell.Offset(0,1).Address(external:=True ), _
Len(ProductCell.Offset(0,1)), _
Chr(34) & ProductCell.Offset(0,1) & Chr(34)

You can see Debug.Print output in the Immediate Window by pressing ctrl+G.


wrote:
is there a better method than Mid to find the nth number in a
string of numbers and return that value as a number?


No. And I thought you want to return a count, not "that value" (the nth
number). Moreover, your original function did something else as well,
something I do not understand at all.

In any case, the following is how I would design your original function. It
might help in the long run. See the important "Notes" following the VBA
code.


Option Explicit

Function TotalCC(productRange As Range, _
Optional ShiftNum As String = "1") As Long
Dim countVals As Long
Dim countFrac As Long
Dim productCell As Range

countVals = 0
countFrac = 0
For Each productCell In productRange
If Mid(productCell.Offset(0, 1), 4, 1) = ShiftNum Then
countVals = countVals + 1
ElseIf Len(productCell.Offset(0, 9)) < 4 Then
countFrac = countFrac + 1
End If
Next
TotalCC = WorksheetFunction.Round(countVals + countFrac / 3, 0)
End Function


Usage:
=totalCC('1x2'!$A$1:$A$100,3)


Notes:

1. Option Explicit

Forces to declare the type of every variable. A "good practice" to catch
typos like Week v. WeekNum early on.


2. productRange As Range

By passing the range, you obviate the need for Application.Volatile. It is
also more flexible if the location or size of the range A1:A100 might
change.

Application.Volatile is "evil". It is necessary in some circumstances. But
it should be avoid whenever possible. Using a "volatile" function causes
all cells that reference it and their dependents, direct and indirect, to be
recalculated every time Excel does __any__ recalculation. That can become
time-consuming.


3. Optional shiftNum As String = "1"

If a parameter is optional, I feel there should be a useful default. Your
original implementation return zero if the parameter were missing. Not
useful.

I use type String to make the later comparison more reliable; that is, to
avoid multiplying Mid(...) by 1, which fails if Mid(...) is not numeric.

Note the even though shiftNum is type String, we can pass a number, as
demonstrated by the usage example above. So it does not encumber usage.
But it also allows for non-numeric shifts, e.g. "G" for graveyard.


4. Function totalCC(...) As Long

The advantage of type Long is flexibility. Type Integer is limited to
32767, too small if the size of productRange ever exceeds that number.

There is no reason to use type Integer today, except in rare circumstances
of very large arrays, where size matters.

In particular, there is no longer any performance benefit of Integer v. Long
in modern computers.


5. countFrac = countFrac + 1
[....]
totalCC = ... countVals + countFrac / 3

Your original implementation has:

CountVals = CountVals + (1 / 3)

I don't know why. But it is unreliable to add 1/3 iteratively. It is more
reliable to count the number of times (n) we would add 1/3, then add n/3 at
the end.

For example:

Sub testit1()
Dim x As Variant, i As Long
x = 1
For i = 1 To 3: x = x + 1 / 3: Next
MsgBox (x = 2)
End Sub

displays False(!) even though we add 1/3 three times.

The reason is complicated; it has to do with non-integer representation and
arithmetic.

The problem is compounded by using type Long for countVals, as I do. In my
case, adding 1/3 and storing into a non-Double variable each iteration is
the same as adding zero(!).

(You used implicit type Variant. So VBA will coerce countVals to type
Double after adding 1/3.)


6. TotalCC = WorksheetFunction.Round(...)

Again, I do not know why you add 1/3 sometimes.

But by doing so in your original implementation, CountVals (type Variant)
might be a non-integer.

So when you do TotalCC = CountVals, VBA implicitly rounds CountVals, since
TotalCC is type Integer.

If that is your intent, it is arguably better to use
WorksheetFunction.Round.

The issue is: VBA Round(2.5) is not the same as Excel ROUND(2.5,0). If you
want Excel rounding in VBA, as most people do, use WorksheetFunction.Round.

Alternatively, you might have intended:

totalCC = Int(countVals + countFrac/3)
or
totalCC = WorksheetFunction.RoundUp(countVals + countFrac/3, 0)


An aside, no longer relevant....

7. If IsMissing(ShiftNum) Then
Else

Previously, I suggested that alternative because I assumed there is some
code between the If-Then and Else statements in your full implementation,
omitted from the posted code snippet.

If not, of course you should simply write:

If Not IsMissing(ShiftNum) Then

followed by everything you put after the Else statement. There is no need
for an Else statement.


Whew! Hope all that helps.