PDA

View Full Version : [SOLVED] What is wrong with my VBA code?



Sandler
05-26-2016, 01:17 PM
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
05-26-2016, 01:34 PM
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
05-26-2016, 01:39 PM
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
05-26-2016, 02:54 PM
@ 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
05-27-2016, 01:13 AM
@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
05-27-2016, 03:23 AM
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
05-27-2016, 04:22 AM
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
05-27-2016, 06:23 AM
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 :banghead::banghead::banghead: hasn't shaken things loose yet.

Sandler
05-27-2016, 06:29 AM
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/showthread.php?56159-hide-rows-based-on-highlighted-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
05-27-2016, 06:43 AM
@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
05-27-2016, 06:47 AM
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
05-27-2016, 08:04 AM
http://programmer.97things.oreilly.com/wiki/index.php/Write_Code_for_Humans_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
05-27-2016, 08:43 AM
or

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

SamT
05-27-2016, 09:06 AM
Is EntireRow needed?

= * Not(it.Hidden)

mdmackillop
05-27-2016, 10:12 AM
Is EntireRow needed?

= * Not(it.Hidden)
I guess not!

Aussiebear
05-27-2016, 11:07 PM
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
05-28-2016, 03:41 AM
I've always admired SNB's minimalist code: short and to the point. With regard to For each Cell, the OP here (http://www.vbaexpress.com/forum/showthread.php?56170-If-range-contans-quot-0-quot-mark-as-incomplete) wants 23.4 million cells tested, surely not the approach to consider.

SamT
05-28-2016, 06:04 AM
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.

:beerchug: :friends:

mikerickson
05-28-2016, 10:47 AM
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
06-02-2016, 10:50 PM
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 (http://www.vbaexpress.com/forum/showthread.php?56170-If-range-contans-quot-0-quot-mark-as-incomplete) 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 D1:D2000 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.

GTO
06-03-2016, 02:45 AM
Excellent link at #12 Paul :thumb

The only thing I sort of disagreed with was minimizing comments. Admittedly though, my exception would be for postings, as quite often I have seen thank-you's from various OP's when at least the harder bits are explained.

Mark