Review VBA code? What is this in English?

The first one that's true is calculated and the following ones are ignored.

Range("D2").Activate
    Select Case ActiveCell.Value ' Examine the contents of D2
    
        Case Is > 1.15 * IEHaworthLogin
           ' It's greater than 1.15 * IEHaworthLogin (I assume that this is some kind of numeric variable)
            Result = 1.15 * IEHaworthLogin
            ActiveCell.Interior.Color = vbRed ' Make the cell red
        Case 1.1 * IEHaworthLogin To 1.15 * IEHaworthLogin
             'The value is between 1.11 * IEHaworthLogin and 1.15 * IEHaworthLogin
            ActiveCell.Interior.Color = vbYellow
        Case 0.9 * IEHaworthLogin To 1.1 * IEHaworthLogin
           'The value is between 0.9 * IEHaworthLogin and 1.11 * IEHaworthLogin
            ActiveCell.Interior.Color = vbGreen
        Case Is < 0.9 * IEHaworthLogin
          ' 0.9 * IEHaworthLogin value is < .9 so make it cyan color
            ActiveCell.Interior.Color = vbCyan
    End Select

Open in new window

SOLUTION

Fabrice Lambert

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

SOLUTION

aikimark

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

It doesn't make sense to have Result = in the first Case or anywhere (if similar statements were added) because Result is being set to what the value already is and you could use Range("D2").Value directly.

ASKER

Looks like the data type in the variable is General... and it is an average. I was at first concerned about the E+02 type values in the average variable but it is not there anymore. I prefer the If and ElseIf method myself... The sub is only checking a couple cells, so it is not a huge deal.. and I did not want to go down the Conditional Formatting path.

There was a comment by Martin that said "rest are ignored"? Is that ok? As long as Result is calculated once it is good? Or should that be in each of the follow Cases too? I guess I am not used to the Case Is and Case method instead of the If and ElseIf

ASKER

How would that look replacing the code with: Range("D2").Value directly? Or using the If and IfElse method instead of CaseIs and Case?

I have no idea what Result is used for but MsgBox Range("D2").Value would show the same value that's in Result.

IMO, If and IF Else would be a poorer way to go.

SOLUTION

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

ASKER CERTIFIED SOLUTION

Bill Prew

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

ASKER

Thanks all for the help today.... guess I need to start writing more Case Is \ Case statements then If and IfElse... Appreciate the help.

You’re welcome and I’m glad I was able to help.

If you expand the “Full Biography” section of my profile you’ll find links to some articles I’ve written that may interest you.

Marty - Microsoft MVP 2009 to 2017
Experts Exchange MVE 2015
Experts Exchange Top Expert Visual Basic Classic 2012 to 2017

ASKER

Maybe I should add this note, that I had to remove the ActiveCell references too.

Based on this the final code looks like:

Select Case (Range("D2").Value)
    Case Is > 1.15 * IEHaworthLogin
        Result = 1.15 * IEHaworthLogin
        Range("D2").Interior.Color = vbRed
    Case 1.1 * IEHaworthLogin To 1.15 * IEHaworthLogin
        Range("D2").Interior.Color = vbYellow
    Case 0.9 * IEHaworthLogin To 1.1 * IEHaworthLogin
        ActiveCell.Interior.Color = vbGreen
    Case Is < 0.9 * IEHaworthLogin
        Range("D2").Interior.Color = vbCyan
End Select

Open in new window

Do you need line 3?