Microsoft Excel Macro Changing the Value of a Cell when Ran

Question

So, I had to make a macro that would run through a report and take all the records that had the same invoice number and put them into one row with its date, who it's sold to, the sum of all the rows of the same invoice number's extended pricing, the order freight if applicable, and the sum of the extended pricing and order freight.

It would take these values and put them into a new sheet.

Here is where I'm running in to a problem. For the most part, it seems like the macro works but once it gets to a certain record, it actually changed the invoice of the number i.e. (123456) to the next row's (i.e. 123457) Thus, the invoice that was 123456 is no longer existing and is instead grouped together with 123457.

I stepped through it, and there should have been no reason for the cell to change value, as it did not do it for any other cells. Does anyone have a reason as to why this is doing it?

Here's my code. Thanks in advance.

    Sub CombineInvoice()

    Dim InvoiceNum As String, InvoiceDate As String, SoldToAcct As String
    Dim ExtPrice As Double, OrderFreight As Double, OrderTotal As Double
    Dim WS2 As Worksheet

    Application.ScreenUpdating = False

    Set WS2 = Sheets.Add

    With Sheet1.Range("A1,C1,D1,BB1,BD1")
        .Copy Destination:=Sheet2.Range("A1:F1")
    End With

    Sheet1.Select

    InvoiceNum = Range("A2").Value
    InvoiceDate = Range("C2").Value
    SoldToAcct = Range("D2").Value

    Sheet2.Select
    Selection.Offset(0, 5).Value = "Order Total"
    Range("A2").Select
    Sheet1.Select
    Range("A2").Select

    Do
        'Set order freight
        OrderFreight = OrderFreight + Selection.Offset(0, 55)
        'Set invoice date
        InvoiceDate = Selection.Offset(0, 2)
        'Set Sold to account
        SoldToAcct = Selection.Offset(0, 3)
        'Get extended price
        Do Until Selection.Value <> InvoiceNum
            ExtPrice = ExtPrice + Selection.Offset(0, 53)
            'Make sure orderFreight is the same
            If (OrderFreight <> OrderFreight Or OrderFreight = 0) Then
                OrderFreight = OrderFreight + Selection.Offset(0, 55)
            Else
                OrderFreight = OrderFreight
            End If

            Selection.Offset(1, 0).Select
        Loop
        'Add Extended Price to Order Freight
        OrderTotal = ExtPrice + OrderFreight
        'Populate Sheet 2 with data
        Sheet2.Select
        With Selection
             .Value = InvoiceNum
             .Offset(0, 1).Value = InvoiceDate
             .Offset(0, 2).Value = SoldToAcct
             .Offset(0, 3).Value = ExtPrice
             .Offset(0, 4).Value = OrderFreight
             .Offset(0, 5).Value = OrderTotal
             .Offset(1, 0).Select
        End With
        'Return to sheet 1 for next invoice number
        Sheet1.Select
        Selection.Value = Selection.Offset(1, 0)
        InvoiceNum = Selection.Value
        ExtPrice = 0
        OrderFreight = 0
        If (Selection.Value = "") Then Exit Do

    Loop

    Application.ScreenUpdating = True

    End Sub

I just gave those as examples. But they are similarly sequential. — lessthanthree, Jun 15 '12 at 19:16
Two things. 1) Using selection is VBA is a 100% bad idea! (unless ABSOLUTELY necessary, or for debugging). You are much better off referring to range or cells. 2) It's hard to test without actual data and all the selection references, but if you change the lines "Selection.Value = Selection.Offset(1, 0) | InvoiceNum = Selection.Value" to just "InvoiceNum = Selection.Offset(1)", does that fix it. — Scott Holtzman, Jun 15 '12 at 19:33
Ahhh. I figured it out @alan I had an unnecessary statement towards the end. I commented out the SelectionValue=Selection.Offset(1,0) and it ran through just fine. Thank you for your interest in my question, though! — lessthanthree, Jun 15 '12 at 19:35
@Scott I have never used VBA before and I was tasked with coming up with this macro. >.< If there is a better way to referencing the cells, I am 100% open to it. I honestly just don't know any better, so if you could point me to somewhere that could help me improve my code, that would be very much appreciate. :D — lessthanthree, Jun 15 '12 at 19:37
For some ideas to avoid `Select` see http://stackoverflow.com/a/10717999/445425 — chris neilsen, Jun 15 '12 at 21:41
@lessthanthree, please put your comment in as an answer and mark it as *the* answer so that we don't get left with loads of open questions. It is fine to answer your own questions. — Julian Knight, Jun 26 '12 at 22:30

Gaffi · Accepted Answer · 2012-08-03T13:31:35.390

Near the end of the first/outer loop, we have these lines of code:

Sheet1.Select
Selection.Value = Selection.Offset(1, 0)
InvoiceNum = Selection.Value
ExtPrice = 0
OrderFreight = 0
If (Selection.Value = "") Then Exit Do

The OP indicated in comments that removing this line worked:

Selection.Value = Selection.Offset(1, 0)

However, since Selection.Value is used on the very next line, I propose the following change to the next line as well (this was also suggested by @Scott in the comments).

InvoiceNum = Selection.Offset(1, 0).Value

Microsoft Excel Macro Changing the Value of a Cell when Ran

1 Answers1