Error with FOR/IF statement

andrewb90

Well-known Member
Joined
Dec 16, 2009
Messages
1,077
Hello all,

I know I have an issue with the statement, but I'm not entirely sure how to correct it. Any guidance would be greatly appreciated.

Code:
    Dim r As Long    For r = 4 To 50
        If r <> 6 And r <> 45 Then
[COLOR=#ff0000]            If Range("C" & r).Value = "Front" Then Range("C" & r: "G" & r).select[/COLOR]
            With Selection
                .HorizontalAlignment = xlGeneral
                .VerticalAlignment = xlBottom
                .WrapText = False
                .Orientation = 0
                .AddIndent = False
                .IndentLevel = 0
                .ShrinkToFit = False
                .ReadingOrder = xlContext
                .MergeCells = True
            End With
   
        End If
    Next r
Red is the problem. What I am trying to do is if "C" matches that value, then C:G will be selected. Additionally If anybody can assist me with putting multiple values as a search instead of just "front", i'd like to add several others, I'm just not sure how.
 
A few issues:
Code:
[COLOR=#ff0000]If Range("C" & r).Value = "Front" Then Range("C" & r: "G" & r).select[/COLOR]
should be:
Code:
[COLOR=#ff0000]If Range("C" & r).Value = "Front" Then Range("C" & r & ":G" & r).select[/COLOR]
and your "With" block is NOT contained in your IF ... THEN block, meaning that section is going to be running on your last selected range, even when the condition is not met.

I would also make the following changes.
You should really try to avoid using SELECTs in your VBA code. It is usually unnecessary and slows your code down.
I also prefer using the Cells(row,column) range reference when building range references like you are.
So, try this variation:
Code:
    Dim r As Long
    Dim myRange As Range
    
    For r = 4 To 50
        If (r <> 6) And (r <> 45) Then
            If Range("C" & r).Value = "Front" Then
                Set myRange = Range(Cells(r, "C"), Cells(r, "G"))
                With myRange
                    .HorizontalAlignment = xlGeneral
                    .VerticalAlignment = xlBottom
                    .WrapText = False
                    .Orientation = 0
                    .AddIndent = False
                    .IndentLevel = 0
                    .ShrinkToFit = False
                    .ReadingOrder = xlContext
                    .MergeCells = True
                End With
            End If
        End If
    Next r
 
Upvote 0
Thank you! It works perfectly!
2 questions: 1, since I have quite a few with selections in my code, switching them out will with CELLS will speed up my code? does that apply in all cases or just when I have big list of changes like this code?
2nd: I tried doing OR and my 'back' value but that gave an error, do I need to search those values separately or can it be done all in one?
 
Upvote 0
since I have quite a few with selections in my code, switching them out will with CELLS will speed up my code? does that apply in all cases or just when I have big list of changes like this code?
A few simple tips to help speed up your code:
- Remove all instances of Select, Selection, ActiveCell, and Activate where you can (not always possible, but most of the time it is)
- Avoid loops, when possible
- Use Application.ScreenUpdating = False at the top of your code and then Application.ScreenUpdating = True at the bottom. This will suppress all the screen-flickering as changes are being made and present all changes to the screen just once at the very end

I tried doing OR and my 'back' value but that gave an error, do I need to search those values separately or can it be done all in one?
It would need to look something like this (using my preferred Cells(r,c) method):
Code:
If (Cells(r,"C") = "Front") Or (Cells(r,"C") = "Back") Then
Note that each statement on either side of the OR must be a complete check (cannot do If Cell(r,"C") = "Front" Or "Back" Then)
 
Last edited:
Upvote 0
Thank you very much. That is very useful information! I'm trying to become better and better at this (as well as learning some other languages)

I do disable Screen updating at the start of my code, but what would the display alerts do? Would Userforms and Msg boxes still show?

I do have tons of loops and running certain code takes a long time. I'm not sure if this would need to be a separate question or not but this is a loop I use very frequently, and on a few different ranges:
Code:
Rows("5:128").Hidden = False    For r = 5 To 129
        If r <> 5 And r <> 24 And r <> 43 And r <> 62 And r <> 81 And r <> 110 Then
            If Range("D" & r).Value = "" Then Rows(r).Hidden = True
        End If
    Next r

Is there are better alternative?

Thank you so much for the help!
 
Upvote 0
I do disable Screen updating at the start of my code, but what would the display alerts do? Would Userforms and Msg boxes still show?
Sorry, that was a typo on my part. I meant to say Application.ScreenUpdating. I have gone back and fixed it. DisplayAlerts is what you use to suppress warning or error messages.

I do have tons of loops and running certain code takes a long time. I'm not sure if this would need to be a separate question or not but this is a loop I use very frequently, and on a few different ranges:
There are times when loops are necessary, like if each row has to be evaluated independently of the other rows. There really isn't getting around that.

However, I have seen instances like this (where you want to add columns A and B and put in column C):
Code:
For r = 1 to 100
    Cells(r,"C").Formula = "=A" & r & " + B" & r
Next r
that could simply be written like this:
Code:
Range(Cells(1, "C"), Cells(100, "C")).FormulaR1C1 = "=RC[-2]+RC[-1]"
 
Upvote 0
No problem!
You are welcome.
 
Upvote 0
Got a follow up question. The with the current code, is there a way to do something to each of the items that are not selected? so basically the rows that don't qualify with my search terms and get merged, I would like to put a formula in column G using something like this:
Code:
Set myForm = Range(Cells(r, "G"))
The challenge is applying it to all the other rows. I was thinking ELSE, but that seems it would only do one or the other. I can simply use my same code to review the same range and look at different criteria and get my formulas, I was just wondering if there was perhaps a more elegant way to do both at the same time instead of going over the same range of cells multiple times.
 
Upvote 0
Can you be a little more specific as to exactly what it is you are trying to do?
Maybe walk us through an actual example?
 
Upvote 0

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top