0

I have 3 Sheets: Work, Bill, and Cust. Cust column A contains my unique customers, which I then paste onto cell A3 on the Work sheet where it runs its calculations and then paste it on to the Bill sheet. I then take the next value on the Cust sheet and i paste it back to Work, run the calculation and paste it below the previous set on the Bill sheet. I have 2 questions.

Why isn't my loop working? I'm trying to keep going until I run out of customers on the cust sheet? Why is it that I can use the custom range BillPlace in the first part of my code, yet I actually have to refer to the cells in the later parts? Thanks in advance

Sub test1()
    Dim WorkPlace As Range, BillPlace As Range, WorkProd As Range

    Set WorkPlace = Sheets("Work").Cells(3, 1)
    Set BillPlace = Sheets("Bill").Cells(3, 1)
    Set WorkProd = WorkPlace.CurrentRegion

    WorkPlace.CurrentRegion.Copy
    BillPlace.PasteSpecial xlPasteAll

    Sheets("Cust").Select
    Cells(1, 1).Copy
    WorkPlace.PasteSpecial xlPasteAll

    WorkProd.Copy
    Sheets("Bill").Select
    Range("A3").Select
    Selection.End(xlDown).Select
    Selection.Offset(1, 0).PasteSpecial xlPasteAll
    Sheets("Cust").Select
    Cells(2, 1).Select
    Selection.Offset(1, 0).Select

    Do
        ActiveCell.Offset(1, 0).Copy
        WorkPlace.PasteSpecial xlPasteAll
        WorkProd.Copy
        Sheets("Bill").Select
        Range("A3").Select
        Selection.End(xlDown).Select
        Selection.Offset(1, 0).PasteSpecial xlPasteAll
    Loop Until IsEmpty(ActiveCell.Offset(1, 0))
End Sub
James Chen
  • 237
  • 2
  • 6
  • 18

3 Answers3

2

@Portland Runner has a point about using a For Each / Next loop. By doing that you can probably eliminate the counters and a bunch of selecting from your working code above, removing a bunch of complexity from your process.

The principle of a For/Next loop is easy enough: define TheLargerRange containing the cells you will loop through. Define a SingleCell range to contain the current cell you are working with. Then you can start the loop saying something like:

For Each SingleCell in TheLargerRange 
    '~~> your loop actions go here 
Next SingleCell

Also, you can do a lot without selecting specific locations in your workbook. Instead copy, paste, or assign values by just referencing the location. If you want, you can set variables to make this easier in longer code.

The following example just moves a column of customer data from one sheet to another, as an example of how to use the For Each / Next loop structure and how to avoid selecting everything you work with. There is only one selection in this code, and that is only because the compiler chokes if you use End(xldown) to attempt setting a range on an unselected tab. Otherwise there could be no selections.

Sub UsingForNextAndAvoidingSelections()

    '~~> Set variables for referencing the "Cust" tab
        Dim CustomerList As Range
        Dim Customer As Range
        Dim CustomerTab As Worksheet

        Set CustomerTab = Sheets("Cust")
        CustomerTab.Select
        Set CustomerList = CustomerTab.Range("A1", Range("A1").End(xlDown))

    '~~> Set variables for referencing the "Bill" tab
        Dim BillTab As Worksheet
        Dim BillRow As Range

        Set BillTab = Sheets("Bill")
        Set BillRow = BillTab.Range("A1")

    '~~> Loop through the customer list, copying each value to the new BillRow location
        For Each Customer In CustomerList
            Customer.Copy
            BillRow.PasteSpecial xlPasteAll
            Set BillRow = BillRow.Offset(1, 0)
        Next Customer

End Sub

12/27/2013: I just realized why the code Set CustomerList = CustomerTab.Range("A1", Range("A1").End(xlDown)) was throwing an error when CustomerTab was not selected: I forgot to fully qualify the second range statement in that line: Range("A1").End(xlDown).

I believe that if you qualify that line of code like this Set CustomerList = CustomerTab.Range("A1", CustomerTab.Range("A1").End(xlDown)) you can eliminate the CustomerTab.Select that precedes it and conduct the entire process without a single Select.

Instant Breakfast
  • 1,383
  • 2
  • 14
  • 28
1
     WorkProd.Copy
    Sheets("Bill").Select
    Range("A3").Select
    Selection.End(xlDown).Select
    Selection.Offset(1, 0).PasteSpecial xlPasteAll
Loop Until IsEmpty(ActiveCell.Offset(1, 0))

You are going to the end of a column and pasting one row further down. You then check if the cell one row further down is empty, but it won't be because you've just pasted into it. This is why it repeats endlessly.

I assume you should be looking for an empty cell somewhere other than one row below the current cursor position.

Andy G
  • 19,232
  • 5
  • 47
  • 69
  • Duh, i feel so stupid right now. Ok so i basically need to reference back to the "cust" sheet where i grabbed my original value, then offset until itsempty. i should set the last value as a range right? – James Chen Oct 15 '13 at 18:53
  • Sorry in advance So i set PlaceHolder as a range and its just not doing it for me. and god i gotta learn how to respond in this forum Do ActiveCell.Offset(1, 0).Select Set PlaceHolder = ActiveCell Selection.Copy WorkPlace.PasteSpecial xlPasteAll WorkProd.Copy Sheets("Bill").Select Range("A3").Select Selection.End(xlDown).Select Selection.Offset(1, 0).PasteSpecial xlPasteAll PlaceHolder.Select Loop Until IsEmpty(PlaceHolder.Offset(1, 0)) – James Chen Oct 15 '13 at 19:07
  • Click on **help** under the **Add Comment** button (god i gotta learn how to respond in this forum) – Steve Oct 15 '13 at 19:21
0

HA! i fixed it. This isn't the most orthodox approach but it worked. Oh pardon me but i did it in production so the name of the sheets and cell positions changed slightly. CountC is a helper cell that counts the number of customers. Thanks everyone for your help.

Sub Pull_Billing()
    Dim WorkPlace As Range, BillPlace As Range, WorkProd As Range, PlaceHolder As Range, CountC As Integer, n As Integer


    Set WorkPlace = Sheets("Work").Cells(3, 1)
    Set BillPlace = Sheets("ABS_Billing_Sheet").Cells(3, 1)
    Set WorkProd = WorkPlace.CurrentRegion
    CountC = Sheets("CTA_Info").Cells(1, 5).Value


    Sheets("CTA_info").Cells(2, 2).Copy
    WorkPlace.PasteSpecial xlPasteAll
    WorkPlace.CurrentRegion.Copy
    BillPlace.PasteSpecial xlPasteAll
    Sheets("CTA_Info").Select
    Cells(3, 2).Copy
    WorkPlace.PasteSpecial xlPasteAll


    WorkProd.Copy
    Sheets("ABS_Billing_Sheet").Select
    Range("A3").Select
    Selection.End(xlDown).Select
    Selection.Offset(1, 0).PasteSpecial xlPasteAll


    Sheets("CTA_Info").Select
    Cells(4, 2).Select
    n = ActiveCell.Row
    Do
    Cells(n, 2).Select
    Selection.Copy
    WorkPlace.PasteSpecial xlPasteAll
    WorkProd.Copy
    Sheets("ABS_Billing_Sheet").Select
    Range("A3").Select
    Selection.End(xlDown).Select
    Selection.Offset(1, 0).PasteSpecial xlPasteAll
    Sheets("CTA_Info").Select
    Cells(n + 1, 2).Select
    n = ActiveCell.Row
    Loop Until n > CountC + 2


    Sheets("CTA_info").Cells(2, 2).Copy
    WorkPlace.PasteSpecial xlPasteAll


    Sheets("ABS_Billing_Sheet").Select



    End Sub
James Chen
  • 237
  • 2
  • 6
  • 18