What is wrong with my VBA code?

[Sandler]

In the range D1 to D2000, if the cell is filled in light blue, I want the row to be either hidden or showing based on the sub procedure that is ran. I used a function to get the specific RGB. Unfortunately, it is not doing anything with the hiding or showing of rows.

Thanks.

Sub HideRows()
Dim Rw As Range


Worksheets("Revenue Sheet").Select


For Each Rw In Range("D1:D2000")
    If Rw.Interior.Color = RGB(0, 176, 240) Then
        Rw.EntireRow.Hidden = True
    End If
Next Rw


End Sub




Sub ShowRows()
Dim Rw As Range


Worksheets("Revenue Sheet").Select


For Each Rw In Range("D1:D2000")
    If Rw.Interior.Color = RGB(0, 176, 240) Then
        Rw.EntireRow.Hidden = False
    End If
Next Rw


End Sub

[snb]

Sub M_snb() 
  For Each it In sheets("Revenue Sheet").Range("D1:D2000") 
    it.EntireRow.Hidden = it.Interior.Color = RGB(0, 176, 240) 
  Next 
End Sub

If none of the cells has color RGB(0, 176, 240) then nothing will happen.
So please check : msgbox it.interior.color

NB these 3 lines of code replace both your macros.

[Sandler]

Thanks for the quick response. Must have been something with my worksheet reference. I took the worksheet reference out and moved the actual code into the sheet that I wanted it to work on and it finally started working. Thanks again

[SamT]

@ snb,

NB these 3 lines of code replace both your macros.

That implies that Hidden Ranges don't have an Interior, but it is still available for when they are visible.

Or something like that.



@ Sandler, selecting something does not, in itself, cause any following code to refer to or act upon the selection.

OTOH:

Sub HideRows() 
    Dim Cel As Range      
     
    With Sheets("Revenue Sheet")     
    For Each Cel In .Range("D1:D2000") 'note dot before Range
        If Cel.Interior.Color = RGB(0, 176, 240) Then Cel.EntireRow.Hidden = True 
    Next Rw 
    End With      
End Sub

Or, shorter by two lines:

For Each Cel In Sheets("Revenue Sheet").Range("D1:D2000")

And finally

Dim LR as Long
With Sheets("Revenue Sheet") 
LR = .Cells(Rows.Count, "D").End(xlUp).Row 'note dot before Cells
   For Each Cel In .Range("D1:D" & LR) 
'
'
'

[snb]

@SamT


On the contrary:

Sub M_snb() 
    For Each it In sheets("Revenue Sheet").Range("D1:D2000") 
        it.EntireRow.Hidden = it.Interior.Color = RGB(0, 176, 240) 
    Next 
End Sub

Means that actual (the ever present) color of the interior of the cell in column D determines whether the corresponding row is being hidden or not.
The color doesn't change, the visibility otf the row does.

[Aussiebear]

Actually snb I find your code to be a little to crude. Where as Sam's code, and in particular this section;

Sub HideRows() 
    Dim Cel As Range 
     
    With Sheets("Revenue Sheet") 
        For Each Cel In .Range("D1:D2000") 'note dot before Range
            If Cel.Interior.Color = RGB(0, 176, 240) Then Cel.EntireRow.Hidden = True 
        Next Rw 
    End With 
End Sub

A lot more logical for the human brain to read, understand and manage. You really need to think about the capacity of the audience who frequent this forum.

[snb]

Instead of your qualification, in the programming world the term 'elegant' would be used.
It depends on your familiarity with the field.
If you want to learn you will have to be confronted with the unknown. Learning is never 'easy', it takes a lot of effort.
If you don't want to learn you satisfy yourself with a 'solution' (understood or not understood).
The incoming request from the OP suggests you were wrong.

[SamT]

On the contrary:

In that case, I am trying to see how that toggles the hidden attribute.

If this is still True for the cells just hidden:

it.Interior.Color = RGB(0, 176, 240)

Then how does

it.EntireRow.Hidden = True

unhide the row?


I know that I suffer mental blockages but hasn't shaken things loose yet.

[Sandler]

I appreciate all your responses. This forum is a goldmine for learning.
I didn't realize that I already had another response from p45cal which is great as well.

http://www.vbaexpress.com/forum/show...cell-in-column

Only, recently have i started even understanding how to input code through VBA online videos and many different books.

Now, you guys are really speeding up my understanding of excel development and I am very grateful for that.

I hope I am not wasting your time with this lengthy post, but I really want you to understand how you are making a positive difference for me.

Thanks again

[snb]

@SamT

By changing the interiorcolor you can determine whether a row will be hidden or not.
To change the interiorcolor you can use some other VBA code.

[SamT]

Ted, There is nothing wrong with his code style, but I also suspect this piece should be

If it.Interior.Color = X Then it.EntireRow.Hidden = Not it.Hidden

In order to Toggle visibility.

IMO, we should be expose our guests to many diverse styles. I always hope that between my style of verbosity and snb's terseness, we have encompassed the entire spectrum.

The good and the bad of VBA is that the Compiler will digest the beautiful and the ugly and turn both into pretty darn efficient code.

I would really hate to have to parse one of his large projects. His style reminds me of an olde coders drinking game: Write very terse (working) code with obscure functions and see who could figure out what it did. I bet he won many free drinks.

[Paul_Hossler]

http://programmer.97things.oreilly.c...s_not_Machines


I'm just naturally wordy and old school, so I always

1. spell out everything

2. Dim my variables as to what they really are

3. Don't use default properties

4. Don't try to be too clever since I won't remember what I was doing 6 months ago

[mdmackillop]

or

it.EntireRow.Hidden = (it.Interior.Color = RGB(0, 176, 240)) * Not (it.EntireRow.Hidden)

[SamT]

Is EntireRow needed?

= * Not(it.Hidden)

[mdmackillop]

Is EntireRow needed?

= * Not(it.Hidden)

I guess not!

[Aussiebear]

Ted, There is nothing wrong with his code style

There is a lot wrong with snb's code style. Its too abbreviated, and assumes everyone has his intellect. We simply don't have that capacity to understand cryptic code ( and nor should we).

I agree with you Sam, that since we construct the range (For each cell in Range) to test, then we need to apply the test (If cell value equals....). We write code like this ensure that others who following our path will understand what we intended.

Therefore, I find snb's code uninteresting to learn from or use.

[mdmackillop]

I've always admired SNB's minimalist code: short and to the point. With regard to For each Cell, the OP here wants 23.4 million cells tested, surely not the approach to consider.

[SamT]

Its too abbreviated,

If he was the only teacher here, his style would be inappropriate. Being that he is one of many, he gives the intense geek a chance to really learn. I have spent the time in the Help files to understand some of his code and it gets easier after a while. It's not for everybody, but the neither is my style, which is geared to beginners and production level code.

[mikerickson]

The only heuristic change that I would make to snb's code would be the adding of parenthesis.

it.EntireRow.Hidden = (it.Interior.Color = RGB(0, 176, 240))

[Aussiebear]

I've always admired SNB's minimalist code: short and to the point.

Malcom, with the greatest of respect, your admiration is clearly misplaced. Yes snb's code is short, but all too often its not to the benefit of the OP. So whose benefit is it? Surely not to the 95% of the people who frequent this forum. snb & I have discussed this point before, and I asked him to comment his code so that others can learn from the structure and intent. Do you see the commenting? No?, well I'm not surprised because neither do we, for its never there. Does it clearly fail one of the fundamental principles of this forum.... yes it does. We are supposed to be trying to encourage people to learn and understand vba, not intellectual twaddle.

With regard to For each Cell, the OP here wants 23.4 million cells tested, surely not the approach to consider.

Malcolm this part of your post here is clearly misleading. The OP of this thread only wants to test cells within the range D12000 to see if the cell colour is light blue. If so then hide the row. A rather simple set the range, determine the value to be tested and the action to be taken if true.