Smell my code

C

Craneo

New Member
Joined
Oct 21, 2002
Messages
26
Hi.

I've written a macro to populate an array and copy it to another worksheet. It checks the value of the first five elements in the array against what already exists in the destination sheet on a row by row basis.

What I'm trying to do is insert a new row in the destination sheet if there isn't a match but I can't seem to manage this ... I keep getting 'Next without For' errors and such like. It's probably very elemental but it's 01:47am here and I can't seem to engage my brain ..

Where have I erred ?

Code: 
Private Sub CommandButton1_Click()

        'Macro written 23/10/02 by OC
        'declares the variables used in the procedure
        
Dim myDate As Date, myMonth, mySStart, mySEnd, publOK As Integer
Dim myPeriod, myDist, myTeam As String
Dim rng As Range

        'stops the screen showing the macro in progess
        
Application.ScreenUpdating = False

myDate = Worksheets("Input").Range("Date")
myMonth = Month(myDate)
mySStart = Worksheets("Input").Range("Shift_Start")
mySEnd = Worksheets("Input").Range("Shift_End")
myPeriod = Worksheets("Input").Range("Period")
myDist = Worksheets("Input").Range("District")
myTeam = Worksheets("Input").Range("Team_No.")

        'Checks to see if data exists on this row for the same date, shift period and team.
        'If data exists and the user wants to overwrite
        'then overwrite it.

For Each objCell In Worksheets(Format(myMonth * 30, "MMMM")).Range("A4:A65535")
        If objCell.Value = myDate And _
            objCell.Offset(0, 1).Value = mySStart And _
               objCell.Offset(0, 2).Value = mySEnd And _
                  objCell.Offset(0, 3).Value = myPeriod And _
                    objCell.Offset(0, 4).Value = myDist And _
                        objCell.Offset(0, 5).Value = myTeam Then
                        
                        
         Select Case MsgBox("This shift already contains data for your team." & Chr(13) _
         & Chr(13) & "Overwrite?", vbYesNo _
                  + vbQuestion + vbDefaultButton2, "Daily Sales Report")
            
                Case vbYes
                    
                    cnt = -1
                    For Each addr In Array("A2", "B2", "C2", "D2", "E2", "F2", _
                    "G2", "C9", "B17", "C17", "D17", "B18", "C18", "D18", "B19", "C19", "D19", _
                    "B28", "C28", "D28", "E28", "F28", "G28", "B29", "C29", "D29", "E29", "F29", "G29", _
                    "B30", "C30", "D30", "E30", "F30", "G30", "B31", "C31", "D31", "E31", "F31", "G31", _
                    "J3", "K3", "L3", "M3", "J4", "K4", "L4", "M4", "J5", "K5", "L5", "M5", _
                    "J6", "K6", "L6", "M6", "K11", "K12", "K17", "K18", "K23", "K24", "K29", "K30", _
                    "K35", "K36", "K41", "K42", "K43", "I47", "K47", "I51")
                    
                    cnt = cnt + 1
                    objCell.Offset(0, cnt).Value = Worksheets("Input").Range(addr).Value
                    
                    Next
                    
                    publOK = 1
       
                    Exit For
                    
                
                Case vbNo
                
                    Exit For
                    
        End Select
        
        Else
           
        'Publishes the data on the next available row on the target sheet
Publish:
        Set rng = Worksheets(Format(myMonth * 30, "MMMM")).Range("A65536").End(xlUp).Offset(1, 0)
        
        cnt = -1
        For Each addr In Array("A2", "B2", "C2", "D2", "E2", "F2", _
        "G2", "C9", "B17", "C17", "D17", "B18", "C18", "D18", "B19", "C19", "D19", _
        "B28", "C28", "D28", "E28", "F28", "G28", "B29", "C29", "D29", "E29", "F29", "G29", _
        "B30", "C30", "D30", "E30", "F30", "G30", "B31", "C31", "D31", "E31", "F31", "G31", _
        "J3", "K3", "L3", "M3", "J4", "K4", "L4", "M4", "J5", "K5", "L5", "M5", _
        "J6", "K6", "L6", "M6", "K11", "K12", "K17", "K18", "K23", "K24", "K29", "K30", _
        "K35", "K36", "K41", "K42", "K43", "I47", "K47", "I51")
        
            cnt = cnt + 1
            rng.Offset(0, cnt).Value = Worksheets("Input").Range(addr).Value
        
        Next
        publOK = 1
       
        Exit For

                         
           
Next objCell
 

Excel Facts

Best way to learn Power Query?
Read M is for (Data) Monkey book by Ken Puls and Miguel Escobar. It is the complete guide to Power Query.
Sort by date Sort by votes
On 2002-10-23 20:45, Craneo wrote:
Hi.

I've written a macro to populate an array and copy it to another worksheet. It checks the value of the first five elements in the array against what already exists in the destination sheet on a row by row basis.

What I'm trying to do is insert a new row in the destination sheet if there isn't a match but I can't seem to manage this ... I keep getting 'Next without For' errors and such like. It's probably very elemental but it's 01:47am here and I can't seem to engage my brain ..

Where have I erred ?

Code: 
Private Sub CommandButton1_Click()

        'Macro written 23/10/02 by OC
        'declares the variables used in the procedure
        
Dim myDate As Date, myMonth, mySStart, mySEnd, publOK As Integer
Dim myPeriod, myDist, myTeam As String
Dim rng As Range

        'stops the screen showing the macro in progess
        
Application.ScreenUpdating = False

myDate = Worksheets("Input").Range("Date")
myMonth = Month(myDate)
mySStart = Worksheets("Input").Range("Shift_Start")
mySEnd = Worksheets("Input").Range("Shift_End")
myPeriod = Worksheets("Input").Range("Period")
myDist = Worksheets("Input").Range("District")
myTeam = Worksheets("Input").Range("Team_No.")

        'Checks to see if data exists on this row for the same date, shift period and team.
        'If data exists and the user wants to overwrite
        'then overwrite it.

For Each objCell In Worksheets(Format(myMonth * 30, "MMMM")).Range("A4:A65535")
        If objCell.Value = myDate And _
            objCell.Offset(0, 1).Value = mySStart And _
               objCell.Offset(0, 2).Value = mySEnd And _
                  objCell.Offset(0, 3).Value = myPeriod And _
                    objCell.Offset(0, 4).Value = myDist And _
                        objCell.Offset(0, 5).Value = myTeam Then
                        
                        
         Select Case MsgBox("This shift already contains data for your team." & Chr(13) _
         & Chr(13) & "Overwrite?", vbYesNo _
                  + vbQuestion + vbDefaultButton2, "Daily Sales Report")
            
                Case vbYes
                    
                    cnt = -1
                    For Each addr In Array("A2", "B2", "C2", "D2", "E2", "F2", _
                    "G2", "C9", "B17", "C17", "D17", "B18", "C18", "D18", "B19", "C19", "D19", _
                    "B28", "C28", "D28", "E28", "F28", "G28", "B29", "C29", "D29", "E29", "F29", "G29", _
                    "B30", "C30", "D30", "E30", "F30", "G30", "B31", "C31", "D31", "E31", "F31", "G31", _
                    "J3", "K3", "L3", "M3", "J4", "K4", "L4", "M4", "J5", "K5", "L5", "M5", _
                    "J6", "K6", "L6", "M6", "K11", "K12", "K17", "K18", "K23", "K24", "K29", "K30", _
                    "K35", "K36", "K41", "K42", "K43", "I47", "K47", "I51")
                    
                    cnt = cnt + 1
                    objCell.Offset(0, cnt).Value = Worksheets("Input").Range(addr).Value
                    
                    Next
                    
                    publOK = 1
       
                    Exit For
                    
                
                Case vbNo
                
                    Exit For
                    
        End Select
        
        Else
           
        'Publishes the data on the next available row on the target sheet
Publish:
        Set rng = Worksheets(Format(myMonth * 30, "MMMM")).Range("A65536").End(xlUp).Offset(1, 0)
        
        cnt = -1
        For Each addr In Array("A2", "B2", "C2", "D2", "E2", "F2", _
        "G2", "C9", "B17", "C17", "D17", "B18", "C18", "D18", "B19", "C19", "D19", _
        "B28", "C28", "D28", "E28", "F28", "G28", "B29", "C29", "D29", "E29", "F29", "G29", _
        "B30", "C30", "D30", "E30", "F30", "G30", "B31", "C31", "D31", "E31", "F31", "G31", _
        "J3", "K3", "L3", "M3", "J4", "K4", "L4", "M4", "J5", "K5", "L5", "M5", _
        "J6", "K6", "L6", "M6", "K11", "K12", "K17", "K18", "K23", "K24", "K29", "K30", _
        "K35", "K36", "K41", "K42", "K43", "I47", "K47", "I51")
        
            cnt = cnt + 1
            rng.Offset(0, cnt).Value = Worksheets("Input").Range(addr).Value
        
        Next
        publOK = 1
       
        Exit For

                         
           
Next objCell
Click to expand...

this code appears twice


Next

publOK = 1

Exit For

but you only have one

For Each objCell In Worksheets(Format(myMonth * 30, "MMMM")).Range("A4:A65535")

EDIT: now that my printer has been refilled with paper that is not the problem :oops:

Edit 2: was close you have two "For Each" statements and three "Next" closures.
_________<font size=+1.<fontcolor="Blue">Sam</font size=+1.</fontcolor="Blue">
Why is it that the nuttiest people define reality?
This message was edited by SamS on 2002-10-23 21:32
This message was edited by SamS on 2002-10-23 21:37
 
Upvote 0
Thanks for the advice Sam. You put me on the right track. What I had to do was put the last element the section following the 'Else' statement OUTSIDE the loop. The loop was checking for one condition the match, if a match wasn't found the procedure should move on to the copy data to the next line.

It works now ..... except for one thing :rolleyes: I want an error handler to check to see if a Date value has been entered on the first worksheet. I've tried

If Worksheets("Input").Range("Date") = "" Then MsgBox "Valid Date required"

GoTo Completed

End If

But get an End If without Block If error ... agggh!!

Without this handler, you get strange data appearing in the other monthly sheets.

Weirdness .....
 
Upvote 0
I think that if the Completed procedure is outside the IF .... End If statement will do this.
 
Upvote 0
I don't know if it is possible but with Application.ScreenUpdating = False why not make the Sheet Active?
 
Upvote 0
You must log in or register to reply here.

Similar threads

G
  • Question
Copy Range of Cells based on Criteria, Paste it on rows below it, Do This Until All conditions are met, Then Delete All Copied Rows
Replies
0
Views
219
glaccounting
G
G
  • Question
Combining / Reducing Multiple Formulas down to 1 Formula
Replies
3
Views
315
Bosquedeguate
Bosquedeguate
SkywardPalm
  • Question
VBA: Copy & Paste information from another template
Replies
2
Views
375
My Aswer Is This
My Aswer Is This
K
  • Solved
Excel Percentage formulas and Pivot table chart displays
3 4 5
Replies
48
Views
2K
kellym9236
K
A
  • Question
ignore zero value when copy the firs three column & the last three columns
Replies
0
Views
246
abdo meghari
A

Forum statistics

Threads
1,214,897
Messages
6,122,141
Members
449,066
Latest member
Andyg666

Share this page

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top