0

In the code below, the loop keeps looping even when the condition is met. When I run it line by line and initially run the loop, I can see where the cell selection starts at J300, and I can see the selection as it moves down the column, but when it gets to the cell where the condition is met, the selection does not offset anymore, but the code continues to loop and it does not go to the next line. What am I doing wrong and what can I do to make it better? Thanks for your help.

Sub RollForwardDataImport()
Dim rcell5, rcell6, rcell7, rcell8, rcell9 As Long
  rcell8 = 0
  rcell5 = Range("F2").Value
  rcell9 = Range("F2").Value
  Workbooks.Open "\\Inventory\CA-2016.xlsx"
  Worksheets("Total Year").Activate
  Range("J300").Select

  Do While Selection.Value <> recell5
    If Selection.Value = rcell5 Then 
      ActiveCell.Offset(0, 0).Select
    Else: ActiveCell.Offset(1, 0).Select
    End If
  Loop
  ActiveCell.Offset(0, -2).Select
  rcell6 = Selection.Value
  ActiveCell.Offset(0, -4).Select
  rcell7 = Selection.Value
  rcell8 = rcell6 + rcell7 + rcell8

  Workbooks("Import.xlsm").Activate
  Range("H20").Select
  ActiveCell.Value = rcell8
  Workbooks("CA-2016.xlsx").Close SaveChanges:=False
End Sub
Rdster
  • 1,846
  • 1
  • 16
  • 30
Kisheon
  • 43
  • 7
  • 1
    [Avoiding the use of `.Select/`.Activate`](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros) will greatly improve the code, and also teach you how to "tighten up" the code. I highly suggest reading, and applying, ways to avoid it. It may even help solve your issue. – BruceWayne Dec 19 '16 at 15:39
  • 3
    Your `Do While` is testing against recell5 not rcell5. Using `Option Explicit` at the beginning of your module will highlight those mistakes right away. – Rdster Dec 19 '16 at 15:39
  • Ah! YEs! Thank you – Kisheon Dec 19 '16 at 15:40
  • 1
    Also, you only declared rcell9 as a long, all the others are Variants. – Rdster Dec 19 '16 at 15:41
  • 2
    As @Rdster pointed out, you have a typo. I suggest adding `Option Explicit` to the *very top* (outside your `Sub RollForward...`) which will force you to declare all used variables, and would point out your typo. Also, `Dim x, y, z as Long` only sets `z` as `Long`. The others are `Variant`. You need to do `Dim x as Long, y as Long, ...` – BruceWayne Dec 19 '16 at 15:41
  • Thank you.. I will start using that as part of my coding – Kisheon Dec 19 '16 at 15:48
  • I am now getting a type mismatch when at `rcell5 = Range("F2").Value`. A date is in that cell. Even when I Dim rcell5 As Date. What can I try differently? – Kisheon Dec 19 '16 at 15:58
  • Try `Dim rcell5 as String` – BruceWayne Dec 19 '16 at 16:01
  • Awesome! The code ran to the end. I have to add one more major part and may have another question, which I will post separately. Thank you. – Kisheon Dec 19 '16 at 16:18
  • Just to reiterate - while your code is working (yay!) it's **highly** recommended to avoid using `.Select`/`.Activate`. Before you post your new question, I recommend trying to remove the use of those, and then post. – BruceWayne Dec 19 '16 at 16:27
  • @BruceWayne, what might I use instead or would I have to restructure my code. I am novice in VBA. – Kisheon Dec 19 '16 at 16:35

1 Answers1

1

Here's a tweaked version of your code. Note that it avoids using .Select/.Activate and also uses a loop, through a range, to find your values.

I highly suggest stepping through with F8 in order to see how the code works, line by line.

Option Explicit
Sub RollForwardDataImport()
Dim rcell5 As Date, rcell6 As Long, rcell7 As Long, rcell8 As Long, rcell9 As String
rcell8 = 0
rcell5 = 11/1/2016 'Range("F2").Value
rcell9 = Range("F2").Value

Workbooks.Open "\\Inventory\CA-2016.xlsx"

Dim myRng As Range, cel As Range

With Worksheets("Total Year")
    Set myRng = Range("J300:J1000")    'change this to your need. May be farther than row 1000

    For Each cel In myRng
        If cel.Value = rcell5 And Month(cel.Vlaue) = Month(rcell5) Then
            rcell6 = cel.Offset(0, -2).Value
            rcell7 = cel.Offset(0, -4).Value
            rcell8 = rcell6 + rcell7 + rcell8
            rcell5 = rcell5 + 
            Exit For
        Else
           rcell5 = cell.Offset(1,0).Value
        End If
    Next cel
End With

With Workbooks("Import.xlsm")
    .Range("H20").Value = rcell8
End With

Workbooks("CA-2016.xlsx").Close SaveChanges:=False
End Sub

It should work as you wanted, but I may have missed something. If so, let me know.

BruceWayne
  • 22,923
  • 15
  • 65
  • 110