More Efficient code than this

Here is a piece of code I'm using to hide rows that have an "x" in two of the
columns. Is there a more efficient way to write this.

thanks

Sub HideRows()
Application.ScreenUpdating = False

Dim rng As Range
Dim i As Long

Set rng = Range("E5:E204")
i = 0 'set up a variable to see if range contains hidden rows

For Each c In rng
If c.EntireRow.Hidden = True Then
c.EntireRow.Hidden = False
i = 1
End If
Next

If i = 1 Then 'if any hidden rows unhide
Application.ScreenUpdating = True
Exit Sub
End If

For Each c In rng 'if no hidden rows unhidden then hide
If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
c.EntireRow.Hidden = True
End If
Next
Application.ScreenUpdating = True
End Sub

I am a bit confused by what is going on here. You unhide hidden rows, but
exit if there were any, then you hide rows with x x in? Is that right?

--
HTH

Bob Phillips

"thom hoyle" <[email protected]> wrote in message
news:[email protected]...
> Here is a piece of code I'm using to hide rows that have an "x" in two of
the
> columns. Is there a more efficient way to write this.
>
> thanks
>
> Sub HideRows()
> Application.ScreenUpdating = False
>
> Dim rng As Range
> Dim i As Long
>
> Set rng = Range("E5:E204")
> i = 0 'set up a variable to see if range contains hidden rows
>
> For Each c In rng
> If c.EntireRow.Hidden = True Then
> c.EntireRow.Hidden = False
> i = 1
> End If
> Next
>
> If i = 1 Then 'if any hidden rows unhide
> Application.ScreenUpdating = True
> Exit Sub
> End If
>
> For Each c In rng 'if no hidden rows unhidden then hide
> If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> c.EntireRow.Hidden = True
> End If
> Next
> Application.ScreenUpdating = True
> End Sub
>
>

This will be vaguely faster but there are not really enough rows here for
there to be any great difference... I have not clocked it to see if and or
how much this improves things.

Sub HideRows()
Application.ScreenUpdating = False

Dim rng As Range
Dim c As Range
Dim strFirstAddress As String

Set rng = Range("E5:E204")

rng.EntireRow.Hidden = False

Set c = rng.Find("x", , , xlWhole)

If Not c Is Nothing Then
strFirstAddress = c.Address
Do
If c.Offset(0, 3).Value = "x" Then c.EntireRow.Hidden = True
Set c = rng.FindNext(c)
Loop Until c.Address = strFirstAddress
End If

Application.ScreenUpdating = True
End Sub

HTH

"thom hoyle" wrote:

> Here is a piece of code I'm using to hide rows that have an "x" in two of the
> columns. Is there a more efficient way to write this.
>
> thanks
>
> Sub HideRows()
> Application.ScreenUpdating = False
>
> Dim rng As Range
> Dim i As Long
>
> Set rng = Range("E5:E204")
> i = 0 'set up a variable to see if range contains hidden rows
>
> For Each c In rng
> If c.EntireRow.Hidden = True Then
> c.EntireRow.Hidden = False
> i = 1
> End If
> Next
>
> If i = 1 Then 'if any hidden rows unhide
> Application.ScreenUpdating = True
> Exit Sub
> End If
>
> For Each c In rng 'if no hidden rows unhidden then hide
> If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> c.EntireRow.Hidden = True
> End If
> Next
> Application.ScreenUpdating = True
> End Sub
>
>

I am kind of with you on this one Bob. It is not 100% evident what is going
on but doing a find and find next should be a little quicker (not much in
this case) shouldn't it? if anyone would know... you would...

"Bob Phillips" wrote:

> I am a bit confused by what is going on here. You unhide hidden rows, but
> exit if there were any, then you hide rows with x x in? Is that right?
>
> --
> HTH
>
> Bob Phillips
>
> "thom hoyle" <[email protected]> wrote in message
> news:[email protected]...
> > Here is a piece of code I'm using to hide rows that have an "x" in two of
> the
> > columns. Is there a more efficient way to write this.
> >
> > thanks
> >
> > Sub HideRows()
> > Application.ScreenUpdating = False
> >
> > Dim rng As Range
> > Dim i As Long
> >
> > Set rng = Range("E5:E204")
> > i = 0 'set up a variable to see if range contains hidden rows
> >
> > For Each c In rng
> > If c.EntireRow.Hidden = True Then
> > c.EntireRow.Hidden = False
> > i = 1
> > End If
> > Next
> >
> > If i = 1 Then 'if any hidden rows unhide
> > Application.ScreenUpdating = True
> > Exit Sub
> > End If
> >
> > For Each c In rng 'if no hidden rows unhidden then hide
> > If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> > c.EntireRow.Hidden = True
> > End If
> > Next
> > Application.ScreenUpdating = True
> > End Sub
> >
> >
>
>
>

It looks like you want to toggle between rows being hidden and rows being
not hidden. If so
This may be closer to what your code does.

Sub HideRows()
Application.ScreenUpdating = False
Dim rng1 as Range
Dim rng As Range
Dim c As Range
Dim cnt as Long
Dim strFirstAddress As String

Set rng = Range("E5:E204")
On Error Resume Next
set rng1 = rng.SpecialCells(xlVisible)
On Error goto 0
if rng1 is nothing then
cnt = 0
else
cnt = rng1.cnt
end if
if rng.Count <> cnt then
rng.EntireRow.Hidden = False
Else

Set c = rng.Find("x", , , xlWhole)

If Not c Is Nothing Then
strFirstAddress = c.Address
Do
If c.Offset(0, 3).Value = "x" Then c.EntireRow.Hidden = True
Set c = rng.FindNext(c)
Loop Until c.Address = strFirstAddress
End If
End if

Application.ScreenUpdating = True
End Sub

--
Regards,
Tom Ogilvy


"Jim Thomlinson" <[email protected]> wrote in message
news:[email protected]...
> This will be vaguely faster but there are not really enough rows here for
> there to be any great difference... I have not clocked it to see if and or
> how much this improves things.
>
> Sub HideRows()
> Application.ScreenUpdating = False
>
> Dim rng As Range
> Dim c As Range
> Dim strFirstAddress As String
>
> Set rng = Range("E5:E204")
>
> rng.EntireRow.Hidden = False
>
> Set c = rng.Find("x", , , xlWhole)
>
> If Not c Is Nothing Then
> strFirstAddress = c.Address
> Do
> If c.Offset(0, 3).Value = "x" Then c.EntireRow.Hidden = True
> Set c = rng.FindNext(c)
> Loop Until c.Address = strFirstAddress
> End If
>
> Application.ScreenUpdating = True
> End Sub
>
> HTH
>
> "thom hoyle" wrote:
>
> > Here is a piece of code I'm using to hide rows that have an "x" in two
of the
> > columns. Is there a more efficient way to write this.
> >
> > thanks
> >
> > Sub HideRows()
> > Application.ScreenUpdating = False
> >
> > Dim rng As Range
> > Dim i As Long
> >
> > Set rng = Range("E5:E204")
> > i = 0 'set up a variable to see if range contains hidden rows
> >
> > For Each c In rng
> > If c.EntireRow.Hidden = True Then
> > c.EntireRow.Hidden = False
> > i = 1
> > End If
> > Next
> >
> > If i = 1 Then 'if any hidden rows unhide
> > Application.ScreenUpdating = True
> > Exit Sub
> > End If
> >
> > For Each c In rng 'if no hidden rows unhidden then hide
> > If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> > c.EntireRow.Hidden = True
> > End If
> > Next
> > Application.ScreenUpdating = True
> > End Sub
> >
> >

In my view the fastest method is the one where you hide all the rows together
like that shown below.
On my computer if I hide each row separately, it took 20 seconds but using
the code below it took 1 second. The reading of the value of the cells takes
the longest amount of time in spreadsheet operations and hence I have
sometimes copied the entire range into memory at one go and then checked
values etc in that array.

Sub HideRows()
Dim i%, rng As Range
Set rng = Sheet1.Cells(1, 2)
For i = 2 To 10000
If Sheet1.Cells(i, 2) = "x" And Sheet1.Cells(i, 3) = "x" Then
Set rng = Union(rng, Sheet1.Cells(i, 2))
End If
Next i
rng.Rows.EntireRow.Hidden = True
End Sub

Alok Joshi

"thom hoyle" wrote:

> Here is a piece of code I'm using to hide rows that have an "x" in two of the
> columns. Is there a more efficient way to write this.
>
> thanks
>
> Sub HideRows()
> Application.ScreenUpdating = False
>
> Dim rng As Range
> Dim i As Long
>
> Set rng = Range("E5:E204")
> i = 0 'set up a variable to see if range contains hidden rows
>
> For Each c In rng
> If c.EntireRow.Hidden = True Then
> c.EntireRow.Hidden = False
> i = 1
> End If
> Next
>
> If i = 1 Then 'if any hidden rows unhide
> Application.ScreenUpdating = True
> Exit Sub
> End If
>
> For Each c In rng 'if no hidden rows unhidden then hide
> If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> c.EntireRow.Hidden = True
> End If
> Next
> Application.ScreenUpdating = True
> End Sub
>
>

Hi,
Building on Jim's solution here. Instead of hiding rows one by one, you can
gather all rows into a single range within the loop then hide in a single
statement. This should speed up the process.
- Add a range variable: rgResult as Range
- replace
If c.Offset(0, 3).Value = "x" Then c.EntireRow.Hidden = True
by
If c.Offset(0, 3).Value = "x" Then
if rgResult is nothing then
set rgResult=c
else
set rgResult=application.union(rgResult,c)
end if
end if
- finally, hide the range's rows at the end, right before the
application.screenupdating:
if not rgResult is Nothing then
rgResult.EntireRow.Hidden = True
end if
Application.ScreenUpdating = True
End Sub
--
Regards,
Sébastien
"Jim Thomlinson" wrote:

> This will be vaguely faster but there are not really enough rows here for
> there to be any great difference... I have not clocked it to see if and or
> how much this improves things.
>
> Sub HideRows()
> Application.ScreenUpdating = False
>
> Dim rng As Range
> Dim c As Range
> Dim strFirstAddress As String
>
> Set rng = Range("E5:E204")
>
> rng.EntireRow.Hidden = False
>
> Set c = rng.Find("x", , , xlWhole)
>
> If Not c Is Nothing Then
> strFirstAddress = c.Address
> Do
> If c.Offset(0, 3).Value = "x" Then c.EntireRow.Hidden = True
> Set c = rng.FindNext(c)
> Loop Until c.Address = strFirstAddress
> End If
>
> Application.ScreenUpdating = True
> End Sub
>
> HTH
>
> "thom hoyle" wrote:
>
> > Here is a piece of code I'm using to hide rows that have an "x" in two of the
> > columns. Is there a more efficient way to write this.
> >
> > thanks
> >
> > Sub HideRows()
> > Application.ScreenUpdating = False
> >
> > Dim rng As Range
> > Dim i As Long
> >
> > Set rng = Range("E5:E204")
> > i = 0 'set up a variable to see if range contains hidden rows
> >
> > For Each c In rng
> > If c.EntireRow.Hidden = True Then
> > c.EntireRow.Hidden = False
> > i = 1
> > End If
> > Next
> >
> > If i = 1 Then 'if any hidden rows unhide
> > Application.ScreenUpdating = True
> > Exit Sub
> > End If
> >
> > For Each c In rng 'if no hidden rows unhidden then hide
> > If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> > c.EntireRow.Hidden = True
> > End If
> > Next
> > Application.ScreenUpdating = True
> > End Sub
> >
> >

Alok, when running your code only the first row gets hidden whether or not
"x" appears in A1. What could be my mistake.

RobertR

Hi Alok,
Your code contains a variable as i%. Is "% allowed in a variable name?
or it is some kind of accelarator since you have used only i?

>>sometimes copied the entire range into memory at one go and then
checked
values etc in that array<<

Can you pl tell me how to do this?
Thanks
Ketan

It implicitly declares i as an integer.

for example, from the immediate window:

? typename(j%)
Integer
? typename(m$)
String
? typename(n&)
Long
? typename(o!)
Single

--
Regards,
Tom Ogilvy



"Ketan" <[email protected]> wrote in message
news:[email protected]...
> Hi Alok,
> Your code contains a variable as i%. Is "% allowed in a variable name?
> or it is some kind of accelarator since you have used only i?
>
> >>sometimes copied the entire range into memory at one go and then
> checked
> values etc in that array<<
>
> Can you pl tell me how to do this?
> Thanks
> Ketan
>

Perhaps it is because it isn't testing the columns specified and hides the
first row regardless of conditions (but then it was only for demo of the
technique).

Sub HideRows()
Dim i%, rng As Range
For i = 2 To 10000
If Sheet1.Cells(i, "E") = "x" And Sheet1.Cells(i, "H") = "x" Then
if Not rng is nothing then
Set rng = Union(rng, Sheet1.Cells(i, 1))
else
Set rng = Sheet1,Cells(i,1)
End If
Next i
rng.Rows.EntireRow.Hidden = True
End Sub

--
Regards,
Tom Ogilvy



"Robert" <[email protected]> wrote in message
news:[email protected]...
> Alok, when running your code only the first row gets hidden whether or not
> "x" appears in A1. What could be my mistake.
>
> RobertR
>
>
>

Here's are just some additional ideas...

Sub HideRows()
Dim Cell As Range
Const str_x As String = "x"

For Each Cell In Range("E5:E204").Cells
Cell.EntireRow.Hidden = (Cell = str_x) And (Cell(1, 4) = str_x)
Next
End Sub

Sub HideRows_v2()
Dim R As Long '(R)ow
Const str_x As String = "x"

For R = 5 To 204
Rows(R).Hidden = (Cells(R, 5) = str_x) And (Cells(R, 8) = str_x)
Next
End Sub


--
Dana DeLouis
Win XP & Office 2003


"thom hoyle" <[email protected]> wrote in message
news:[email protected]...
> Here is a piece of code I'm using to hide rows that have an "x" in two of
> the
> columns. Is there a more efficient way to write this.
>
> thanks
>
> Sub HideRows()
> Application.ScreenUpdating = False
>
> Dim rng As Range
> Dim i As Long
>
> Set rng = Range("E5:E204")
> i = 0 'set up a variable to see if range contains hidden rows
>
> For Each c In rng
> If c.EntireRow.Hidden = True Then
> c.EntireRow.Hidden = False
> i = 1
> End If
> Next
>
> If i = 1 Then 'if any hidden rows unhide
> Application.ScreenUpdating = True
> Exit Sub
> End If
>
> For Each c In rng 'if no hidden rows unhidden then hide
> If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> c.EntireRow.Hidden = True
> End If
> Next
> Application.ScreenUpdating = True
> End Sub
>
>

Robert,
My mistake, I should have explained that the code hides all rows which have
an x both in columns B and C.
Alok

"Robert" wrote:

> Alok, when running your code only the first row gets hidden whether or not
> "x" appears in A1. What could be my mistake.
>
> RobertR
>
>
>

Thanks Tom.
I was just wondering how to put entire range values in an memory arrey?
and the how to refer and retrive or manipulate the same from arrey.

I have searched the vba help files but not explained properly.

Thanks,
Ketan

Alok, your code works. Timings:-3000 rows 0.40.0, 6000 rows 0.26.9, 12000 rows
2:00.8, 20160 rows 2:00.9

Dana, I tried your code on 3,204 rows it takes 0:18:0 but at each repeat
processing
(ie I unhid the rows and ran again) the time increments by about 7secs. Just
curious why.

Tom, when I copied your code,"..Set rng= Sheet1.Cells(i,1) "E" .." turned
red. Did not proceed.

RobertR


"Dana DeLouis" wrote:

> Here's are just some additional ideas...
>
> Sub HideRows()
> Dim Cell As Range
> Const str_x As String = "x"
>
> For Each Cell In Range("E5:E204").Cells
> Cell.EntireRow.Hidden = (Cell = str_x) And (Cell(1, 4) = str_x)
> Next
> End Sub
>
> Sub HideRows_v2()
> Dim R As Long '(R)ow
> Const str_x As String = "x"
>
> For R = 5 To 204
> Rows(R).Hidden = (Cells(R, 5) = str_x) And (Cells(R, 8) = str_x)
> Next
> End Sub
>
>
> --
> Dana DeLouis
> Win XP & Office 2003
>
>
> "thom hoyle" <[email protected]> wrote in message
> news:[email protected]...
> > Here is a piece of code I'm using to hide rows that have an "x" in two of
> > the
> > columns. Is there a more efficient way to write this.
> >
> > thanks
> >
> > Sub HideRows()
> > Application.ScreenUpdating = False
> >
> > Dim rng As Range
> > Dim i As Long
> >
> > Set rng = Range("E5:E204")
> > i = 0 'set up a variable to see if range contains hidden rows
> >
> > For Each c In rng
> > If c.EntireRow.Hidden = True Then
> > c.EntireRow.Hidden = False
> > i = 1
> > End If
> > Next
> >
> > If i = 1 Then 'if any hidden rows unhide
> > Application.ScreenUpdating = True
> > Exit Sub
> > End If
> >
> > For Each c In rng 'if no hidden rows unhidden then hide
> > If c.Value = "x" And c.Offset(0, 3).Value = "x" Then
> > c.EntireRow.Hidden = True
> > End If
> > Next
> > Application.ScreenUpdating = True
> > End Sub
> >
> >
>
>
>