Can someone point me in the right direction?

Ok, so the code below works with one exception, it gives me a cell value in column J4 which I do not want. I have tried editing to get it to go to the next row but being a novice with VBA I am not 100% understanding what the middle section is doing.
The idea of the macro is that it looks for a word first, upon finding that word it moves over 4 columns to the right and inserts a Vlookup which is referencing another sheet in the workbook.
I think it is the variable part of the code I am not understanding.
Can anyone spot the error and more so explain the variables in the statment.
I added the conditional formatting myself and so understand this aspect.
I think my main lack of understanding is the object use and would really apprecaite some commenting to make it clear what is happening.
Thanks
Patrick

Sub Insert_VLOOKUP()
Dim Findfirst As Object, FindNext As Object, FindNext2 As Object
Set Findfirst = Cells.Find(What:="Cheese", LookIn:=xlValues)
If Not Findfirst Is Nothing Then
Findfirst.Select
With Range("A" & Findfirst.Row & ":F" & Findfirst.Row).Borders(xlEdgeTop)
ActiveCell.Offset(ColumnOffset:=4).Activate
ActiveCell = "=VLOOKUP(RC[-5],'Product per Call '!R3C1:R[596]C[16],16,FALSE)"
End With
Set FindNext2 = Findfirst
Do
Set FindNext = Cells.FindNext(After:=FindNext2)
If Not FindNext Is Nothing Then
With Range("A" & FindNext.Row & ":F" & FindNext.Row).Borders(xlEdgeTop)
ActiveCell.Offset(ColumnOffset:=4).Activate
ActiveCell = "=VLOOKUP(RC[-4],'Product per Call '!R3C1:R[596]C[16],16,FALSE)"
Selection.NumberFormat = "0.000"
Selection.FormatConditions.Delete
Selection.FormatConditions.Add Type:=xlCellValue, Operator:=xlGreaterEqual _
, Formula1:="0.3"
With Selection.FormatConditions(1).Font
.Bold = True
.Italic = False
.ColorIndex = 10
End With
End With
End If
Set FindNext2 = FindNext
FindNext2.Interior.ColorIndex = 0
FindNext2.Select
Loop Until FindNext.Address = Findfirst.Address
End If
End Sub

Hi Patrick,

Sorry, I can't provide a good explanation of the object use b/c I'm not used to working with "objects" so I changed your variables to "ranges" which (as I understand them) are a type (subset) of object.

The code wasn't going to the next row because you use "findfirst.select" which changes the active cell & affects both;
"ActiveCell.Offset(ColumnOffset:=4).Activate
ActiveCell = "=VLOOKUP(RC[-5],'Product per Call '!R3C1:R[596]C[16],16,FALSE)"
"
and in the second section (w/o changing the activecell, so it just moves across the page),
"ActiveCell.Offset(ColumnOffset:=4).Activate
ActiveCell = "=VLOOKUP(RC[-4],'Product per Call '!R3C1:R[596]C[16],16,FALSE)"
"

However, selecting &/or activating cells is not normally needed & slows the macro down so I've modified these lines to allow the removal of the selection & activation.

The following should work except for:
* a probable need to change vlookup formulae strings - should the first one be for "RC[-5]" & the second for "RC[-4]"?
* changing the borders. Your original version didn't seem to do anything to the borders when I tested it so I've commented the 2 lines but left them in place for modification eg:
you may want to replace,
"'.Borders(xlEdgeTop).?"
with the lines,
" With .Borders(xlEdgeTop)
.LineStyle = xlContinuous
.Weight = xlThin
End With
"

btw,
*I've copied the actions which were done in the second part up to the "findfirst" section - just guessing they were overlooked.
*To prevent more errors in where the vlookup is inserted it may be best to change "cells.find" to a set column eg,
change
Set Findfirst = Cells.Find(What:="Cheese", LookIn:=xlValues)
&
Set FindNext1 = Cells.FindNext(After:=FindNext2)
to the appropriate column eg
Set Findfirst = Columns("A:A").Find(What:="Cheese", LookIn:=xlValues)
&
Set FindNext1 = Columns("A:A").FindNext(After:=FindNext2)
*You may find the following site useful for providing different approaches to dealing with certain cells meeting criteria (eg "cheese"):
http://www.rondebruin.nl/delete.htm
(just modify these to insert your vlookup rather than delete the row)

Give it a go, let me know if you have any problems...

Sub Insert_VLOOKUP()
'http://excelforum.com/showthread.php?t=574786
Application.ScreenUpdating = False
Dim Findfirst As Range: Dim FindNext1 As Range: Dim FindNext2 As Range
Set Findfirst = Cells.Find(What:="Cheese", LookIn:=xlValues)
If Findfirst Is Nothing Then GoTo TidyUp
Findfirst.Offset(ColumnOffset:=4).Formula = "=VLOOKUP(RC[-5],'Product per Call '!R3C1:R[596]C[16],16,FALSE)"
With Range("A" & Findfirst.Row & ":F" & Findfirst.Row)
'.Borders(xlEdgeTop).?
.NumberFormat = "0.000"
.FormatConditions.Delete
.FormatConditions.Add Type:=xlCellValue, Operator:=xlGreaterEqual _
, Formula1:="0.3"
With .FormatConditions(1).Font
.Bold = True
.Italic = False
.ColorIndex = 10
End With
End With
Set FindNext2 = Findfirst
'Just temporarily to allow loop to start
Set FindNext1 = Cells(Rows.Count, Columns.Count)
Do Until FindNext1.Address = Findfirst.Address
Set FindNext1 = Cells.FindNext(After:=FindNext2)
If FindNext1 Is Nothing Then GoTo TidyUp
FindNext1.Offset(ColumnOffset:=4).Formula = "=VLOOKUP(RC[-4],'Product per Call '!R3C1:R[596]C[16],16,FALSE)"
With Range("A" & FindNext1.Row & ":F" & FindNext1.Row)
'.Borders(xlEdgeTop).?
.NumberFormat = "0.000"
.FormatConditions.Delete
.FormatConditions.Add Type:=xlCellValue, Operator:=xlGreaterEqual _
, Formula1:="0.3"
With .FormatConditions(1).Font
.Bold = True
.Italic = False
.ColorIndex = 10
End With
End With
Set FindNext2 = FindNext1
FindNext2.Interior.ColorIndex = 0
Loop
TidyUp:
'free memory
Set Findfirst = Nothing: Set FindNext1 = Nothing: Set FindNext2 = Nothing
Application.ScreenUpdating = True
End Sub


hth
Rob Brockett
NZ
Always learning & the best way to learn is to experience...

Cool, so I did some modifications to the code above and now have it working. The original reason for the cell selection was because the only place I wanted to format a cell was the cell where the Vlookup is entered.
Other than that it seems to work exactly the way I want it . I listed the new code below, I moved the conditional format to the end of the macro and just selected a range
Thanks
Patrick

Sub Insert_VLOOKUP_MONDAY_FINAL()
'http://excelforum.com/showthread.php?t=574786
Application.ScreenUpdating = False
Dim Findfirst As Range: Dim FindNext1 As Range: Dim FindNext2 As Range
Set Findfirst = Cells.Find(What:="Cheese", LookIn:=xlValues)
If Findfirst Is Nothing Then GoTo TidyUp
Findfirst.Offset(ColumnOffset:=4).Formula = "=VLOOKUP(RC[-5],'Product per Call'!R3C1:R[596]C[16],16,FALSE)"
With Range("A" & Findfirst.Row & ":F" & Findfirst.Row)
End With
Set FindNext2 = Findfirst
'Just temporarily to allow loop to start
Set FindNext1 = Cells(Rows.Count, Columns.Count)
Do Until FindNext1.Address = Findfirst.Address
Set FindNext1 = Cells.FindNext(After:=FindNext2)
If FindNext1 Is Nothing Then GoTo TidyUp
FindNext1.Offset(ColumnOffset:=4).Formula = "=VLOOKUP(RC[-5],'Product per Call'!R3C1:R[596]C[16],16,FALSE)"
With Range("A" & FindNext1.Row & ":F" & FindNext1.Row)
'.Borders(xlEdgeTop).?
End With
Set FindNext2 = FindNext1
FindNext2.Interior.ColorIndex = 0
Loop
TidyUp:
'free memory
Set Findfirst = Nothing: Set FindNext1 = Nothing: Set FindNext2 = Nothing
Application.ScreenUpdating = True
Range("F4:F209").Select
With Selection
.HorizontalAlignment = xlCenter
.VerticalAlignment = xlBottom
.WrapText = False
.Orientation = 0
.AddIndent = False
.IndentLevel = 0
.ShrinkToFit = False
.ReadingOrder = xlContext
.MergeCells = False
End With
Selection.NumberFormat = "0.000"
Selection.FormatConditions.Delete
Selection.FormatConditions.Add Type:=xlCellValue, Operator:=xlGreaterEqual _
, Formula1:="0.3"
With Selection.FormatConditions(1).Font
.Bold = True
.Italic = False
.ColorIndex = 10
End With
Range("G1").Select
End Sub

Hi Patrick,
Pleased I could help :-)

I've had another play & offer a modified final version:
* I've chopped out any extra lines in your version, these can be deleted as they do nothing eg the 2 "with Range("A" & Find..." statements.
* I'm not sure how long the macro takes to run on your data but two ways to speed it up are by not selecting anything (so I've slightly reworded your final section) & by putting the line "application.screenupdating = true" at the very end of your code (see below).
* I've also deleted some other lines in the final section as I believe that they are default values of Excel & they can therefore be removed based on the assumption that the existing cells already have the default values.
* Wrt "the only place I wanted to format a cell was the cell where the Vlookup is entered.", how do you want it formatted?


Sub Insert_VLOOKUP_MONDAY_Modified_FINAL()
'http://excelforum.com/showthread.php?t=574786
Application.ScreenUpdating = False
Dim Findfirst As Range: Dim FindNext1 As Range: Dim FindNext2 As Range
Set Findfirst = Cells.Find(What:="Cheese", LookIn:=xlValues)
If Findfirst Is Nothing Then GoTo TidyUp
Findfirst.Offset(ColumnOffset:=4).Formula = "=VLOOKUP(RC[-5],'Product per Call'!R3C1:R[596]C[16],16,FALSE)"
Set FindNext2 = Findfirst
'Just temporarily to allow loop to start
Set FindNext1 = Cells(Rows.Count, Columns.Count)
Do Until FindNext1.Address = Findfirst.Address
Set FindNext1 = Cells.FindNext(After:=FindNext2)
If FindNext1 Is Nothing Then GoTo TidyUp
FindNext1.Offset(ColumnOffset:=4).Formula = "=VLOOKUP(RC[-5],'Product per Call'!R3C1:R[596]C[16],16,FALSE)"
Set FindNext2 = FindNext1
FindNext2.Interior.ColorIndex = 0
Loop
TidyUp:
'free memory
Set Findfirst = Nothing: Set FindNext1 = Nothing: Set FindNext2 = Nothing
With Range("F4:F209")
.HorizontalAlignment = xlCenter
.VerticalAlignment = xlBottom
.ReadingOrder = xlContext
.NumberFormat = "0.000"
.FormatConditions.Delete
.FormatConditions.Add Type:=xlCellValue, Operator:=xlGreaterEqual _
, Formula1:="0.3"
With .FormatConditions(1).Font
.Bold = True
.ColorIndex = 10
End With
end with
Range("G1").Select
Application.ScreenUpdating = True
End Sub

hth
Rob Brockett
NZ
always learning & the best way to learn is to experience...