Return to Answer

I wish you would have posted more of your code here. I suspect that the code you posted should be broken out into it's own subroutine and we could have made sure you're using appropriate variable declarations. That being said, I'm only going to address your vlookup method, as you only need one or the other and I don't believe the find method is a good idea.

Per my comment, this code becomes slower because each time you add a formula via Data.Cells(DataRow, 4) = Format((Data.Cells(DataRow, 2) - Result) / Result, "0.00%"), Excel will recalculate all of the formulas in the spreadsheet. Prior to executing this code, you should set calculation to manual with Application.Calculation = xlCalculationManual.

If you don't have an error handler, you will need to add one so you can ensure that calculation is always set back to automatic with Application.Calculation = xlCalculationAutomatic. If you set it to manual, and then an error happens (without a handler), your workbook could cause you some confusion because it's not updating values when you think it should. You would also want to make sure to execute Application.StatusBar = False should any error happen. You don't want the status bar to get "stuck" if an error happens.

###Some other things:

• I'm not a fan of the PascalCase variable names. They should be camelCase to avoid confusion with class properties.

• You don't need = True in IsError(Result) = True. It can simply be If IsError(result) Then.

• That same If statement contains some duplication. It can be simplified as below.

   If (IsError(result)) Or (result = 0) Then
   Data.Cells(dataRow, 3) = "No Old Cost"
   Else
   Data.Cells(dataRow, 3) = Result
   Data.Cells(dataRow, 4) = Format((Data.Cells(dataRow, 2) - Result) / Result, "0.00%")
   End If
   dataRow = dataRow + 1
  

• Note that I also moved the variable incrementor outside of the If block. This also fixes what I believe is a bug that was causing you to skip rows. If you meant to skip rows, increment by two instead.

• You're releasing the CPU to the OS at an inopportune time. Typically, DoEvents should come directly after you update the status bar. If you're concerned about saying it's at 100% while the code is still working, move the update to the end of the loop instead.

• Unless you really feel like that status bar is needed, you should remove it. It's a performance hit as well. It sounds like you do need it though. Processing 70k rows takes some time and you don't want your users to think the program isn't responding.

###The good stuff:

• Thank you for ignoring Microsoft's recommendation to use hungarian notation!
• Your variable names are clear and concise. No confusion about what is what. Well done.

I wish you would have posted more of your code here. I suspect that the code you posted should be broken out into it's own subroutine and we could have made sure you're using appropriate variable declarations. That being said, I'm only going to address your vlookup method, as you only need one or the other and I don't believe the find method is a good idea.

Per my comment, this code becomes slower because each time you add a formula via Data.Cells(DataRow, 4) = Format((Data.Cells(DataRow, 2) - Result) / Result, "0.00%"), Excel will recalculate all of the formulas in the spreadsheet. Prior to executing this code, you should set calculation to manual with Application.Calculation = xlCalculationManual.

If you don't have an error handler, you will need to add one so you can ensure that calculation is always set back to automatic with Application.Calculation = xlCalculationAutomatic. If you set it to manual, and then an error happens (without a handler), your workbook could cause you some confusion because it's not updating values when you think it should. You would also want to make sure to execute Application.StatusBar = False should any error happen. You don't want the status bar to get "stuck" if an error happens.

###Some other things:

• I'm not a fan of the PascalCase variable names. They should be camelCase to avoid confusion with class properties.

• You don't need = True in IsError(Result) = True. It can simply be If IsError(result) Then.

• That same If statement contains some duplication. It can be simplified as below.

   If (IsError(result)) Or (result = 0) Then
   Data.Cells(dataRow, 3) = "No Old Cost"
   Else
   Data.Cells(dataRow, 3) = Result
   Data.Cells(dataRow, 4) = Format((Data.Cells(dataRow, 2) - Result) / Result, "0.00%")
   End If
   dataRow = dataRow + 1
  

• Note that I also moved the variable incrementor outside of the If block. This also fixes what I believe is a bug that was causing you to skip rows. If you meant to skip rows, increment by two instead.

• You're releasing the CPU to the OS at an inopportune time. Typically, DoEvents should come directly after you update the status bar. If you're concerned about saying it's at 100% while the code is still working, move the update to the end of the loop instead.

• Unless you really feel like that status bar is needed, you should remove it. It's a performance hit as well. It sounds like you do need it though. Processing 70k rows takes some time and you don't want your users to think the program isn't responding.

###The good stuff:

• Thank you for ignoring Microsoft's recommendation to use hungarian notation!
• Your variable names are clear and concise. No confusion about what is what. Well done.

I wish you would have posted more of your code here. I suspect that the code you posted should be broken out into it's own subroutine and we could have made sure you're using appropriate variable declarations. That being said, I'm only going to address your vlookup method, as you only need one or the other and I don't believe the find method is a good idea.

Per my comment, this code becomes slower because each time you add a formula via Data.Cells(DataRow, 4) = Format((Data.Cells(DataRow, 2) - Result) / Result, "0.00%"), Excel will recalculate all of the formulas in the spreadsheet. Prior to executing this code, you should set calculation to manual with Application.Calculation = xlCalculateManual (I believe that should be xlCalculationManual - Rob E).

If you don't have an error handler, you will need to add one so you can ensure that calculation is always set back to automatic with Application.Calculation = xlCalculateAutomatic (and that should be xlCalculationAutomatic - Rob E). If you set it to manual, and then an error happens (without a handler), your workbook could cause you some confusion because it's not updating values when you think it should. You would also want to make sure to execute Application.StatusBar = False should any error happen. You don't want the status bar to get "stuck" if an error happens.

###Some other things:

• I'm not a fan of the PascalCase variable names. They should be camelCase to avoid confusion with class properties.

• You don't need = True in IsError(Result) = True. It can simply be If IsError(result) Then.

• That same If statement contains some duplication. It can be simplified as below.

   If (IsError(result)) Or (result = 0) Then
   Data.Cells(dataRow, 3) = "No Old Cost"
   Else
   Data.Cells(dataRow, 3) = Result
   Data.Cells(dataRow, 4) = Format((Data.Cells(dataRow, 2) - Result) / Result, "0.00%")
   End If
   dataRow = dataRow + 1
  

• Note that I also moved the variable incrementor outside of the If block. This also fixes what I believe is a bug that was causing you to skip rows. If you meant to skip rows, increment by two instead.

• You're releasing the CPU to the OS at an inopportune time. Typically, DoEvents should come directly after you update the status bar. If you're concerned about saying it's at 100% while the code is still working, move the update to the end of the loop instead.

• Unless you really feel like that status bar is needed, you should remove it. It's a performance hit as well. It sounds like you do need it though. Processing 70k rows takes some time and you don't want your users to think the program isn't responding.

###The good stuff:

• Thank you for ignoring Microsoft's recommendation to use hungarian notation!
• Your variable names are clear and concise. No confusion about what is what. Well done.

I wish you would have posted more of your code here. I suspect that the code you posted should be broken out into it's own subroutine and we could have made sure you're using appropriate variable declarations. That being said, I'm only going to address your vlookup method, as you only need one or the other and I don't believe the find method is a good idea.

Per my comment, this code becomes slower because each time you add a formula via Data.Cells(DataRow, 4) = Format((Data.Cells(DataRow, 2) - Result) / Result, "0.00%"), Excel will recalculate all of the formulas in the spreadsheet. Prior to executing this code, you should set calculation to manual with Application.Calculation = xlCalculationManual.

If you don't have an error handler, you will need to add one so you can ensure that calculation is always set back to automatic with Application.Calculation = xlCalculationAutomatic. If you set it to manual, and then an error happens (without a handler), your workbook could cause you some confusion because it's not updating values when you think it should. You would also want to make sure to execute Application.StatusBar = False should any error happen. You don't want the status bar to get "stuck" if an error happens.

###Some other things:

• I'm not a fan of the PascalCase variable names. They should be camelCase to avoid confusion with class properties.

• You don't need = True in IsError(Result) = True. It can simply be If IsError(result) Then.

• That same If statement contains some duplication. It can be simplified as below.

   If (IsError(result)) Or (result = 0) Then
   Data.Cells(dataRow, 3) = "No Old Cost"
   Else
   Data.Cells(dataRow, 3) = Result
   Data.Cells(dataRow, 4) = Format((Data.Cells(dataRow, 2) - Result) / Result, "0.00%")
   End If
   dataRow = dataRow + 1
  

• Note that I also moved the variable incrementor outside of the If block. This also fixes what I believe is a bug that was causing you to skip rows. If you meant to skip rows, increment by two instead.

• You're releasing the CPU to the OS at an inopportune time. Typically, DoEvents should come directly after you update the status bar. If you're concerned about saying it's at 100% while the code is still working, move the update to the end of the loop instead.

• Unless you really feel like that status bar is needed, you should remove it. It's a performance hit as well. It sounds like you do need it though. Processing 70k rows takes some time and you don't want your users to think the program isn't responding.

###The good stuff:

• Thank you for ignoring Microsoft's recommendation to use hungarian notation!
• Your variable names are clear and concise. No confusion about what is what. Well done.

I wish you would have posted more of your code here. I suspect that the code you posted should be broken out into it's own subroutine and we could have made sure you're using appropriate variable declarations. That being said, I'm only going to address your vlookup method, as you only need one or the other and I don't believe the find method is a good idea.

Per my comment, this code becomes slower because each time you add a formula via Data.Cells(DataRow, 4) = Format((Data.Cells(DataRow, 2) - Result) / Result, "0.00%"), Excel will recalculate all of the formulas in the spreadsheet. Prior to executing this code, you should set calculation to manual with Application.Calculation = xlCalculateManual.

If you don't have an error handler, you will need to add one so you can ensure that calculation is always set back to automatic with Application.Calculation = xlCalculateAutomatic. If you set it to manual, and then an error happens (without a handler), your workbook could cause you some confusion because it's not updating values when you think it should. You would also want to make sure to execute Application.StatusBar = False should any error happen. You don't want the status bar to get "stuck" if an error happens.

###Some other things:

• I'm not a fan of the PascalCase variable names. They should be camelCase to avoid confusion with class properties.

• You don't need = True in IsError(Result) = True. It can simply be If IsError(result) Then.

• That same If statement contains some duplication. It can be simplified as below.

   If (IsError(result)) Or (result = 0) Then
   Data.Cells(dataRow, 3) = "No Old Cost"
   Else
   Data.Cells(dataRow, 3) = Result
   Data.Cells(dataRow, 4) = Format((Data.Cells(dataRow, 2) - Result) / Result, "0.00%")
   End If
   dataRow = dataRow + 1
  

• Note that I also moved the variable incrementor outside of the If block. This also fixes what I believe is a bug that was causing you to skip rows. If you meant to skip rows, increment by two instead.

• You're releasing the CPU to the OS at an inopportune time. Typically, DoEvents should come directly after you update the status bar. If you're concerned about saying it's at 100% while the code is still working, move the update to the end of the loop instead.

• Unless you really feel like that status bar is needed, you should remove it. It's a performance hit as well. It sounds like you do need it though. Processing 70k rows takes some time and you don't want your users to think the program isn't responding.

###The good stuff:

• Thank you for ignoring Microsoft's recommendation to use hungarian notation!
• Your variable names are clear and concise. No confusion about what is what. Well done.

I wish you would have posted more of your code here. I suspect that the code you posted should be broken out into it's own subroutine and we could have made sure you're using appropriate variable declarations. That being said, I'm only going to address your vlookup method, as you only need one or the other and I don't believe the find method is a good idea.

Per my comment, this code becomes slower because each time you add a formula via Data.Cells(DataRow, 4) = Format((Data.Cells(DataRow, 2) - Result) / Result, "0.00%"), Excel will recalculate all of the formulas in the spreadsheet. Prior to executing this code, you should set calculation to manual with Application.Calculation = xlCalculateManual (I believe that should be xlCalculationManual - Rob E).

If you don't have an error handler, you will need to add one so you can ensure that calculation is always set back to automatic with Application.Calculation = xlCalculateAutomatic (and that should be xlCalculationAutomatic - Rob E). If you set it to manual, and then an error happens (without a handler), your workbook could cause you some confusion because it's not updating values when you think it should. You would also want to make sure to execute Application.StatusBar = False should any error happen. You don't want the status bar to get "stuck" if an error happens.

###Some other things:

• I'm not a fan of the PascalCase variable names. They should be camelCase to avoid confusion with class properties.

• You don't need = True in IsError(Result) = True. It can simply be If IsError(result) Then.

• That same If statement contains some duplication. It can be simplified as below.

   If (IsError(result)) Or (result = 0) Then
   Data.Cells(dataRow, 3) = "No Old Cost"
   Else
   Data.Cells(dataRow, 3) = Result
   Data.Cells(dataRow, 4) = Format((Data.Cells(dataRow, 2) - Result) / Result, "0.00%")
   End If
   dataRow = dataRow + 1
  

• Note that I also moved the variable incrementor outside of the If block. This also fixes what I believe is a bug that was causing you to skip rows. If you meant to skip rows, increment by two instead.

• You're releasing the CPU to the OS at an inopportune time. Typically, DoEvents should come directly after you update the status bar. If you're concerned about saying it's at 100% while the code is still working, move the update to the end of the loop instead.

• Unless you really feel like that status bar is needed, you should remove it. It's a performance hit as well. It sounds like you do need it though. Processing 70k rows takes some time and you don't want your users to think the program isn't responding.

###The good stuff:

• Thank you for ignoring Microsoft's recommendation to use hungarian notation!
• Your variable names are clear and concise. No confusion about what is what. Well done.