0

this is my first time posting. I want to fix a code to color the columns correctly, and also to make the code simpler. Really appreciate your feedbacks.

This is for Excel VBA. Color is coded for the whole columns regardless of if statements.

No error message but the code colors the 2 whole columns red when I want them red if both the 2 corresponding cells in a row have the value.

Sub ColorCol()

Dim i As Long

For i = 2 To Rows.Count

    If Not IsEmpty(Columns("AB").Value) And Not IsEmpty(Columns("CD").Value) Then
        Cells(i, 2).Interior.Color = vbRed
        Cells(i, 3).Interior.Color = vbRed
    End If
    If Not IsEmpty(Columns("PQ").Value) And Not IsEmpty(Columns("RS").Value) Then
        Cells(i, 2).Interior.Color = vbRed
        Cells(i, 3).Interior.Color = vbRed
    End If

Next i

End Sub
braX
  • 11,506
  • 5
  • 20
  • 33
cd3091
  • 67
  • 7
  • 2
    Use `Cells` instead of `Columns`. See [this question](https://stackoverflow.com/questions/11169445/error-in-finding-last-used-cell-in-excel-with-vba) for how to find the last row so you're not looping through all rows. Though conditional formatting could handle this coloring. – BigBen Nov 06 '19 at 17:17
  • Thank you, using Cells helped. – cd3091 Nov 06 '19 at 18:13
  • To make the code simpler, is there a way to combine the 2 if statements and then color them red? – cd3091 Nov 06 '19 at 18:15
  • Yes - you could use `Or`. – BigBen Nov 06 '19 at 18:16
  • Hi BigBen, can you specify where to put Or? I tried to place in 2 ways but errors occurred. – cd3091 Nov 06 '19 at 18:27

2 Answers2

2

You want to check the .Value of each individual cell.

Thus,

If Not IsEmpty(Columns("AB").Value) And Not IsEmpty(Columns("CD").Value)

becomes

If Not IsEmpty(Cells(i, "AB").Value) And Not IsEmpty(Cells(i, "CD").Value)

and similarly for

If Not IsEmpty(Columns("PQ").Value) And Not IsEmpty(Columns("RS").Value)

You could combine the If statements (though I'm not sure it helps with readability). Use parentheses to enclose each condition:

If (Not IsEmpty(Cells(i, "AB").Value) And Not IsEmpty(Cells(i, "CD").Value)) Or _
   (Not IsEmpty(Cells(i, "PQ").Value) And Not IsEmpty(Cells(i, "RS").Value)) Then

More important would be to find the last row and loop to that instead of through all the rows. See this question for how to do that.

BigBen
  • 46,229
  • 7
  • 24
  • 40
  • Keep getting the syntax error starting the first sentence even with parantheses. I looked into the link, thanks! – cd3091 Nov 06 '19 at 19:00
  • Never mind. No error anymore with a space between Or and _ .. Thank you! – cd3091 Nov 06 '19 at 19:04
  • Yes... that is an important space :) – BigBen Nov 06 '19 at 19:05
  • Haha, yeah. It's crazy.. Hope to become better one day like you are. – cd3091 Nov 06 '19 at 19:08
  • Ha, flattering :) – BigBen Nov 06 '19 at 19:13
  • Hi BigBen, I'm trying to find the last row up to where column 1 displays only a number. I used the code in the link that you provided: If Application.WorksheetFunction.CountA(.Cells) <> 0 Then lastrow = .Cells.Find(What:="*", _ After:=.Range("A1"), _ Lookat:=xlPart, _ LookIn:=xlFormulas, _ SearchOrder:=xlByRows, _ SearchDirection:=xlPrevious, _ MatchCase:=False).Row Else lastrow = 1 End If How do I tweak it to do that? Thanks – cd3091 Nov 07 '19 at 14:20
  • So that it doesn't color the rows whose column 1 has non numeric characters. Also, let me know if I'm breaking any rules in the website. – cd3091 Nov 07 '19 at 14:23
  • Got it. Will do. – cd3091 Nov 07 '19 at 14:29
0

If I got your question right, this would be an approach to compare the cells in the same row and colour them red if they contain the same value:

Option Explicit

Sub ColorCol()

Dim i As Long

For i = 2 To 6

    If Cells(i, 2).Value = Cells(i, 3).Value Then
       Cells(i, 2).Interior.Color = vbRed
       Cells(i, 3).Interior.Color = vbRed
    End If

Next i

End Sub
JSRB
  • 2,492
  • 1
  • 17
  • 48
  • I'm trying to combine statements in If and color the cells, so that I don't have to write for coloring after each if statements. – cd3091 Nov 06 '19 at 18:51