PDA

View Full Version : Solved: Excel VBA problem with an "IF". VERY WEIRD



pedrovarela
09-12-2008, 08:45 AM
Hi,

I wrote a simple macro to compare the MarketShare of three products in each region and write down the name of the market leader in each region. The source data is in a sheet in percentages. SOMETHING VERY WEIRD HAPPENS. Sometimes, the ">" doesn't work properly. I have add watches and I can see that the majority of the comparisons work well but sometimes not. It shows that "5% > 20%" ! ARe you familiar with this bug. ANy help will be welcome cause I'm really desperate. The source data is just a list of % and this is my macro. I know I could simplify the "IF" structure but I wanted to make sure I didn't make any error.

Sub ImportFromClickLeader()
Application.ScreenUpdating = False
Dim ARR(3)
For j = 11 To 769
Worksheets("Ugadata").Select
For i = 5 To 3410
ARR(1) = Range("G" & i).Value
ARR(2) = Range("J" & i).Value
ARR(3) = Range("L" & i).Value
If ((ARR(1) > ARR(2)) And (ARR(1) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product1"
ElseIf ((ARR(2) > ARR(1)) And (ARR(2) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product2"
ElseIf ((ARR(3) > ARR(1)) And (ARR(3) > ARR(2))) Then
Worksheets("Model").Range("E" & j).Value = "Product3"
End If
Exit For
Next i
Next j
Application.ScreenUpdating = True
End Sub


Thnaks!!!!!!!

Bob Phillips
09-12-2008, 08:51 AM
I am not understanding what you are saying. Can you post the workbook, and tell us which items are n ot behaving as expected?

pedrovarela
09-12-2008, 10:03 AM
Thanks for the help!

I have attached the Workbook. The name of the script is test. It imports data from the sheet "UGadata".Then searches for the biggest value of the three . The problem is that the output sometimes is not logical! For instance look in the "Model" sheet, in line 35: for the region "74THO", although "Product 1" is the biggest number, the macro chooses "Product 2" as the biggest. It's like if the ">" didn't work properly which makes no sense. Here there's a simplified version of the macro:

Sub test()
Application.ScreenUpdating = False
Dim ARR(3)
Dim j, i As Integer
Dim ougea As String

For j = 2 To 760
ougea = "UGA." & Worksheets("Model").Range("A" & j).Value
Worksheets("Ugadata").Select
For i = 4 To 1128
If (Range("A" & i).Value = ougea) Then
ARR(1) = Range("C" & i).Value
ARR(2) = Range("D" & i).Value
ARR(3) = Range("E" & i).Value
Worksheets("Model").Range("B" & j).Value = ARR(1)
Worksheets("Model").Range("C" & j).Value = ARR(2)
Worksheets("Model").Range("D" & j).Value = ARR(3)
If ((ARR(1) > ARR(2)) And (ARR(1) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product1"
ElseIf ((ARR(2) > ARR(1)) And (ARR(2) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product2"
ElseIf ((ARR(3) > ARR(1)) And (ARR(3) > ARR(2))) Then
Worksheets("Model").Range("E" & j).Value = "Product3"
End If
Exit For

End If
Next i

Next j
Worksheets("Model").Select
Application.ScreenUpdating = True
End Sub

Bob Phillips
09-12-2008, 10:46 AM
It is because your data is text, not numbers. In textm "8.7%" IS greater than "66.7". You need to force them into numbers



Sub test()
Application.ScreenUpdating = False
Dim ARR(3) As Double
Dim j, i As Integer
Dim ougea As String

For j = 2 To 760

Debug.Assert j <> 35

ougea = "UGA." & Worksheets("Model").Range("A" & j).Value
Worksheets("Ugadata").Select
For i = 4 To 1128

If (Range("A" & i).Value = ougea) Then

ARR(1) = CDbl(Replace(Range("C" & i).Value, "%", ""))
ARR(2) = CDbl(Replace(Range("D" & i).Value, "%", ""))
ARR(3) = CDbl(Replace(Range("E" & i).Value, "%", ""))
Worksheets("Model").Range("B" & j).Value = ARR(1)
Worksheets("Model").Range("C" & j).Value = ARR(2)
Worksheets("Model").Range("D" & j).Value = ARR(3)
If ((ARR(1) > ARR(2)) And (ARR(1) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product1"
Rem MsgBox (ARR(1) & ARR(2) & ARR(3) & ougea & "Product1" & j)
ElseIf ((ARR(2) > ARR(1)) And (ARR(2) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product2"
Rem MsgBox (ARR(1) & ARR(2) & ARR(3) & ougea & "Product2" & j)
ElseIf ((ARR(3) > ARR(1)) And (ARR(3) > ARR(2))) Then
Worksheets("Model").Range("E" & j).Value = "Product3"
Rem MsgBox (ARR(1) & ARR(2) & ARR(3) & ougea & "Product3" & j)
End If
Exit For
End If
Next i
Next j
Worksheets("Model").Select
Application.ScreenUpdating = True
End Sub

pedrovarela
09-12-2008, 11:25 AM
Thanks!

I copied pasted your code but I received a run-time error in line 1 "Type Mismatch". The error is in line 13:
ARR(1) = CDbl(Replace(Range("C" & i).Value, "%", ""))

I am running Excel 2003. Do you know what could be the problem?

pedrovarela
09-12-2008, 11:39 AM
I'm trying to analyse why ARR(1) = CDbl(Replace(Range("C" & i).Value, "%", "")) fails. Apparently the error comes from the Replace. I tried to do something like CStr(Range("C" & i).Value) to ensure that we pass a string but I receive the same error.

pedrovarela
09-12-2008, 11:59 AM
Justed tried step by step:
Dim ARR(3) As Double
Dim ARRS(3) As String
...
ARRS(1) = Replace(Range("C" & i).Value, "%", "")
ARR(1) = CDbl(ARRS(1))

and I received the error on the CDbl. Strange?!

Bob Phillips
09-12-2008, 12:12 PM
Try this variation



Sub test()
Application.ScreenUpdating = False
Dim ARR(3) As Double
Dim j, i As Integer
Dim ougea As String

With Worksheets("Ugadata")

For j = 2 To 760

ougea = "UGA." & Worksheets("Model").Range("A" & j).Value
Worksheets("Ugadata").Select
For i = 4 To 1128

If (Range("A" & i).Value = ougea) Then

ARR(1) = CDbl(Replace(.Range("C" & i).Value, "%", "")) / 100
ARR(2) = CDbl(Replace(.Range("D" & i).Value, "%", "")) / 100
ARR(3) = CDbl(Replace(.Range("E" & i).Value, "%", "")) / 100
Worksheets("Model").Range("B" & j).Value = ARR(1)
Worksheets("Model").Range("C" & j).Value = ARR(2)
Worksheets("Model").Range("D" & j).Value = ARR(3)
If ((ARR(1) > ARR(2)) And (ARR(1) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product1"
Rem MsgBox (ARR(1) & ARR(2) & ARR(3) & ougea & "Product1" & j)
ElseIf ((ARR(2) > ARR(1)) And (ARR(2) > ARR(3))) Then
Worksheets("Model").Range("E" & j).Value = "Product2"
Rem MsgBox (ARR(1) & ARR(2) & ARR(3) & ougea & "Product2" & j)
ElseIf ((ARR(3) > ARR(1)) And (ARR(3) > ARR(2))) Then
Worksheets("Model").Range("E" & j).Value = "Product3"
Rem MsgBox (ARR(1) & ARR(2) & ARR(3) & ougea & "Product3" & j)
End If
Exit For
End If
Next i
Next j
End With

Worksheets("Model").Select
Application.ScreenUpdating = True
End Sub

pedrovarela
09-12-2008, 12:22 PM
Unfortunately I Received the same error on:
ARR(1) = CDbl(Replace(.Range("C" & i).Value, "%", "")) / 100

It looks like he doesn't like the CDbl expression. Strange, because if I Store de value as a string and do the Replace, still the CDBl gives an error.

Mavyak
09-12-2008, 02:24 PM
I downloaded the workbook and ran xld's code without incident.

Bob Phillips
09-12-2008, 02:41 PM
Goto Tools>Refernces in the VBIDE, and if any items are marked MISSING, uncheck it.

pedrovarela
09-12-2008, 02:54 PM
Thanks for your patience. It's weird that the CDBL() command doesn't work so it might be something in my options. I check the refereneces and I only have three activated and they look fine:

Visual Basi for Applications
Microsoft Excel 11.0 object library
Ole Automation

ANy other suggestion?

Bob Phillips
09-12-2008, 03:19 PM
Post your offending workbook.

pedrovarela
09-12-2008, 03:20 PM
Just to add that I also tried Val() and same problem

pedrovarela
09-12-2008, 03:21 PM
Here it is. Good-luck!

Bob Phillips
09-12-2008, 03:34 PM
That runs start to finish here, no issue whatsoever.

pedrovarela
09-12-2008, 04:01 PM
I tried again, but I get the same error. It doesn' allow me to make the CDbl(). If it runs well under your Excel I think the problem might be in my Excel configuration. I have Windows 2000 professional and MsExcel2003.

DO you know what options in Excel or in VB should I check to have activated?

Bob Phillips
09-12-2008, 04:51 PM
I have XP and 2003.

There is no option to activate CDbl, it is an intrinsic VBA function. I can't even begin to think why it wouldn't work.

mikerickson
09-12-2008, 05:12 PM
You might use the Val function.
CDbl("20%") errors.
Val("20%") = 20

pedrovarela
09-15-2008, 12:43 AM
Thanks! With eval it works! Thanks!!!!