Hi all

I'm reviewing a production system and trying to implement best
practices.

What I want to do is select a row/range of rows; for EACH row, if ANY
cell is color 'x', I want to run a macro to change all the cells in
that row that ARE color 'x' to color 'y'.

This works:

Sub ChangeColor()
Dim rCell As Range
For Each rCell In Selection.Cells
If Selection.Cells.Interior.ColorIndex = 3 Then
Selection.Cells.Interior.ColorIndex = 1
End If
Next rCell
End Sub

(yeah, I know it's primitive)
but only if ALL the cells in the selection/selected row(s) is/are
color 'x'.

Hence if the user selects entire rows rather than concise selections,
as they are likely to do, the macro will not work, because it's more
than likely that only a portion of the selected row(s) will be color
'x' and the rest will have no fill.

What adjustments do I need to make for the macro to have desired
results whether selections are concise or not?

You're not really looking at row by row, but in this case, it doesn't look like
that's important:

Sub ChangeColor()
Dim rCell As Range
For Each rCell In Selection.Cells
If rcell.Interior.ColorIndex = 3 Then
rcell.Interior.ColorIndex = 1
End If
Next rCell
End Sub

If you're using xl2002+, you may want to look at all the advanced options
(includes changing format) in the Edit|Replace dialog.


prodeji wrote:
>
> Hi all
>
> I'm reviewing a production system and trying to implement best
> practices.
>
> What I want to do is select a row/range of rows; for EACH row, if ANY
> cell is color 'x', I want to run a macro to change all the cells in
> that row that ARE color 'x' to color 'y'.
>
> This works:
>
> Sub ChangeColor()
> Dim rCell As Range
> For Each rCell In Selection.Cells
> If Selection.Cells.Interior.ColorIndex = 3 Then
> Selection.Cells.Interior.ColorIndex = 1
> End If
> Next rCell
> End Sub
>
> (yeah, I know it's primitive)
> but only if ALL the cells in the selection/selected row(s) is/are
> color 'x'.
>
> Hence if the user selects entire rows rather than concise selections,
> as they are likely to do, the macro will not work, because it's more
> than likely that only a portion of the selected row(s) will be color
> 'x' and the rest will have no fill.
>
> What adjustments do I need to make for the macro to have desired
> results whether selections are concise or not?

--

Dave Peterson

On Jun 20, 4:03 pm, Dave Peterson <peter...@verizonXSPAM.net> wrote:
> You're not really looking at row by row, but in this case, it doesn't look like
> that's important:
>
> Sub ChangeColor()
> Dim rCell As Range
> For Each rCell In Selection.Cells
> If rcell.Interior.ColorIndex = 3 Then
> rcell.Interior.ColorIndex = 1
> End If
> Next rCell
> End Sub
>
> If you're using xl2002+, you may want to look at all the advanced options
> (includes changing format) in the Edit|Replace dialog.
>
>
>
>
>
> prodeji wrote:
>
> > Hi all
>
> > I'm reviewing a production system and trying to implement best
> > practices.
>
> > What I want to do is select a row/range of rows; for EACH row, if ANY
> > cell is color 'x', I want to run a macro to change all the cells in
> > that row that ARE color 'x' to color 'y'.
>
> > This works:
>
> > Sub ChangeColor()
> > Dim rCell As Range
> > For Each rCell In Selection.Cells
> > If Selection.Cells.Interior.ColorIndex = 3 Then
> > Selection.Cells.Interior.ColorIndex = 1
> > End If
> > Next rCell
> > End Sub
>
> > (yeah, I know it's primitive)
> > but only if ALL the cells in the selection/selected row(s) is/are
> > color 'x'.
>
> > Hence if the user selects entire rows rather than concise selections,
> > as they are likely to do, the macro will not work, because it's more
> > than likely that only a portion of the selected row(s) will be color
> > 'x' and the rest will have no fill.
>
> > What adjustments do I need to make for the macro to have desired
> > results whether selections are concise or not?
>
> --
>
> Dave Peterson- Hide quoted text -
>
> - Show quoted text -

Dave, thanks for the prompt response,

I tried it, though, and it doesn't seem to be working; is there
perhaps something I'm still missing...?

Actually this was my original code, but it kept telling me 'Cells'
wasn't defined and I couldn't find a solution:

Sub ChangeColor()
Dim rCell As Range, C As Cell
For Each rCell In Selection.Cells
For Each C In rCell.Cells
If C.ColorIndex = 3 Then
C.ColorIndex = 1
End If
Next C
Next rCell
End Sub

I know I may be making some older heads gnash their teeth here

Go back to the code that you were using before--with that suggested change:

Sub ChangeColor()
Dim rCell As Range
For Each rCell In Selection.Cells
If rcell.Interior.ColorIndex = 3 Then
rcell.Interior.ColorIndex = 1
End If
Next rCell
End Sub

For my test workbook, .colorindex = 3 meant that the cell's fill color was red.
..colorindex = 1 meant that the fill was black.

Then select a range that has a few cells that are have a red fill.

Run the macro again and watch the red fill change to black fill.

=======
Your new code has some trouble in it:

> Sub ChangeColor()
> Dim rCell As Range, C As Cell
> For Each rCell In Selection.Cells
> For Each C In rCell.Cells
> If C.ColorIndex = 3 Then
> C.ColorIndex = 1
> End If
> Next C
> Next rCell
> End Sub

There isn't a data type of Cell. You'd want to use: Dim C as Range

Dim rCell as range, c as range
or (I like this better)
dim rCell as range
dim c as range

And if you're looking through each rCell in the selection.cells (one at a time),
then there's no need to loop through that single cell another time.

for each rcell in selection.cells 'one cell at a time
for each c in rcell.cells 'but there's only one cell to loop through

And c.colorindex doesn't exist.
(Using rCell again)
Maybe you wanted the fill:
if rcell.interior.colorindex = 3 then

or maybe you wanted the font color:
if rcell.font.colorindex = 3 then

And remember that if you don't select your range to fix first, your code may
look like it didn't do anything.




prodeji wrote:
>
> On Jun 20, 4:03 pm, Dave Peterson <peter...@verizonXSPAM.net> wrote:
> > You're not really looking at row by row, but in this case, it doesn't look like
> > that's important:
> >
> > Sub ChangeColor()
> > Dim rCell As Range
> > For Each rCell In Selection.Cells
> > If rcell.Interior.ColorIndex = 3 Then
> > rcell.Interior.ColorIndex = 1
> > End If
> > Next rCell
> > End Sub
> >
> > If you're using xl2002+, you may want to look at all the advanced options
> > (includes changing format) in the Edit|Replace dialog.
> >
> >
> >
> >
> >
> > prodeji wrote:
> >
> > > Hi all
> >
> > > I'm reviewing a production system and trying to implement best
> > > practices.
> >
> > > What I want to do is select a row/range of rows; for EACH row, if ANY
> > > cell is color 'x', I want to run a macro to change all the cells in
> > > that row that ARE color 'x' to color 'y'.
> >
> > > This works:
> >
> > > Sub ChangeColor()
> > > Dim rCell As Range
> > > For Each rCell In Selection.Cells
> > > If Selection.Cells.Interior.ColorIndex = 3 Then
> > > Selection.Cells.Interior.ColorIndex = 1
> > > End If
> > > Next rCell
> > > End Sub
> >
> > > (yeah, I know it's primitive)
> > > but only if ALL the cells in the selection/selected row(s) is/are
> > > color 'x'.
> >
> > > Hence if the user selects entire rows rather than concise selections,
> > > as they are likely to do, the macro will not work, because it's more
> > > than likely that only a portion of the selected row(s) will be color
> > > 'x' and the rest will have no fill.
> >
> > > What adjustments do I need to make for the macro to have desired
> > > results whether selections are concise or not?
> >
> > --
> >
> > Dave Peterson- Hide quoted text -
> >
> > - Show quoted text -
>
> Dave, thanks for the prompt response,
>
> I tried it, though, and it doesn't seem to be working; is there
> perhaps something I'm still missing...?
>
> Actually this was my original code, but it kept telling me 'Cells'
> wasn't defined and I couldn't find a solution:
>
> Sub ChangeColor()
> Dim rCell As Range, C As Cell
> For Each rCell In Selection.Cells
> For Each C In rCell.Cells
> If C.ColorIndex = 3 Then
> C.ColorIndex = 1
> End If
> Next C
> Next rCell
> End Sub
>
> I know I may be making some older heads gnash their teeth here

--

Dave Peterson

Hi prodeji,

I've arrived at the same solution as Dave.

Yet, is was a bit slow,
as it was searching all cells in the selected rows,
so I asked in the german groups for a method
to recude the range to be searched.

Thomas Ramel MVP suggested this:

Application.Intersect(Selection, ActiveSheet.UsedRange)

All combined:

Sub ChangeColor()
Dim oCll As Range ' a cell
Dim rCll As Range ' a range of cells
Set rCll = Application.Intersect(Selection, ActiveSheet.UsedRange)
For Each oCll In rCll.Cells
If oCll.Interior.ColorIndex = 3 Then
oCll.Interior.ColorIndex = 1
Else
If oCll.Interior.ColorIndex = 1 Then
oCll.Interior.ColorIndex = 3
End If
End If
Next
End Sub

--
Greetings from Bavaria, Germany

Helmut Weber, MVP WordVBA

Win XP, Office 2003
"red.sys" & Chr$(64) & "t-online.de"

try:

Sub blah()
For Each rw In Selection.Rows
For Each cll In rw.EntireRow.Cells
If cll.Interior.ColorIndex = 3 Then cll.Interior.ColorIndex = 1
Next cll
Next rw
End Sub


This is a quick and dirty solution, it looks at all cells in a row
individually and changes it if required. The rows that it looks at are the
entire rows of whatever cells are selected, even if they're only single cells
on that row. One slight downside is that if the user has selected several
areas (by using the ctrl key while selecting) and one area's rows include
some or all of another area's rows then those rows may be processed more than
once, but if this is a one off process it shouldn't matter.
--
p45cal


"prodeji" wrote:

> Hi all
>
> I'm reviewing a production system and trying to implement best
> practices.
>
> What I want to do is select a row/range of rows; for EACH row, if ANY
> cell is color 'x', I want to run a macro to change all the cells in
> that row that ARE color 'x' to color 'y'.
>
> This works:
>
> Sub ChangeColor()
> Dim rCell As Range
> For Each rCell In Selection.Cells
> If Selection.Cells.Interior.ColorIndex = 3 Then
> Selection.Cells.Interior.ColorIndex = 1
> End If
> Next rCell
> End Sub
>
> (yeah, I know it's primitive)
> but only if ALL the cells in the selection/selected row(s) is/are
> color 'x'.
>
> Hence if the user selects entire rows rather than concise selections,
> as they are likely to do, the macro will not work, because it's more
> than likely that only a portion of the selected row(s) will be color
> 'x' and the rest will have no fill.
>
> What adjustments do I need to make for the macro to have desired
> results whether selections are concise or not?
>
>

Hi Helmut,

I don't think limiting the cells to check to only those selected within the
usedrange is reliable. Reason - entire rows or columns with same format
extend beyond the usedrange to the edge of the sheet.

Regards,
Peter T


"Helmut Weber" <(E-Mail Removed)> wrote in message
news(E-Mail Removed)...
> Hi prodeji,
>
> I've arrived at the same solution as Dave.
>
> Yet, is was a bit slow,
> as it was searching all cells in the selected rows,
> so I asked in the german groups for a method
> to recude the range to be searched.
>
> Thomas Ramel MVP suggested this:
>
> Application.Intersect(Selection, ActiveSheet.UsedRange)
>
> All combined:
>
> Sub ChangeColor()
> Dim oCll As Range ' a cell
> Dim rCll As Range ' a range of cells
> Set rCll = Application.Intersect(Selection, ActiveSheet.UsedRange)
> For Each oCll In rCll.Cells
> If oCll.Interior.ColorIndex = 3 Then
> oCll.Interior.ColorIndex = 1
> Else
> If oCll.Interior.ColorIndex = 1 Then
> oCll.Interior.ColorIndex = 3
> End If
> End If
> Next
> End Sub
>
> --
> Greetings from Bavaria, Germany
>
> Helmut Weber, MVP WordVBA
>
> Win XP, Office 2003
> "red.sys" & Chr$(64) & "t-online.de"

On Jun 20, 7:46 pm, "Peter T" <peter_t@discussions> wrote:
> Hi Helmut,
>
> I don't think limiting the cells to check to only those selected within the
> usedrange is reliable. Reason - entire rows or columns with same format
> extend beyond the usedrange to the edge of the sheet.
>
> Regards,
> Peter T
>
> "Helmut Weber" <nbhymsjxd...@mailinator.com> wrote in message
>
> news(E-Mail Removed)...
>
>
>
> > Hi prodeji,
>
> > I've arrived at the same solution as Dave.
>
> > Yet, is was a bit slow,
> > as it was searching all cells in the selected rows,
> > so I asked in the german groups for a method
> > to recude the range to be searched.
>
> > Thomas Ramel MVP suggested this:
>
> > Application.Intersect(Selection, ActiveSheet.UsedRange)
>
> > All combined:
>
> > Sub ChangeColor()
> > Dim oCll As Range ' a cell
> > Dim rCll As Range ' a range of cells
> > Set rCll = Application.Intersect(Selection, ActiveSheet.UsedRange)
> > For Each oCll In rCll.Cells
> > If oCll.Interior.ColorIndex = 3 Then
> > oCll.Interior.ColorIndex = 1
> > Else
> > If oCll.Interior.ColorIndex = 1 Then
> > oCll.Interior.ColorIndex = 3
> > End If
> > End If
> > Next
> > End Sub
>
> > --
> > Greetings from Bavaria, Germany
>
> > Helmut Weber, MVP WordVBA
>
> > Win XP, Office 2003
> > "red.sys" & Chr$(64) & "t-online.de"- Hide quoted text -
>
> - Show quoted text -

Hi again all

Thanks for the many and obviously thought out responses.
I stepped away from the computer for a bit and came back and was able
to solve it.
Yes, i am feeling more than a bit sheepish
Still, I see some improvements to my newbie solution in the code
suggested by you all.

Many thanks,

prodeji

Thx Peter,

>I don't think limiting the cells to check to only those selected within the
>usedrange is reliable. Reason - entire rows or columns with same format
>extend beyond the usedrange to the edge of the sheet.

right you are.

--
Greetings from Bavaria, Germany

Helmut Weber, MVP WordVBA

Win XP, Office 2003
"red.sys" & Chr$(64) & "t-online.de"