Want to win a PS4? Go Premium and enter to win our High-Tech Treats giveaway. Enter to Win

x
?
Solved

Trouble with my counter in my loop

Posted on 2013-01-21
13
Medium Priority
?
195 Views
Last Modified: 2013-01-21
Hello,

I am getting an error code with this line and not sure why. I am new to loops, so there might be more wrong than I realize.

iRowStart = ImportedSheet.Rows(2) ' This should Row 2 (first row has headers)

My code is looping through the "imported" worksheet and then copying the "approved" (based on the 5th column values) to the "approved worksheet" and the "rejected" rows to the "rejected worksheet". Then I will clear the contents of the imported worksheet (it is a temp worksheet to hold my newly imported data until i separate it).

thanks for any help


Sub SortApprovedRejected()

Dim ApprovedSheet As Worksheet
Dim ImportedSheet As Worksheet
Dim RejectedSheet As Worksheet
Dim iCol As Integer, iRow As Integer, iRowStart As Integer, iRowEnd As Integer, iNextRow As Integer
Dim UniqueIDApprovedWorksheetRng As Range
Dim UniqueIDRejetectedWorksheetRng As Range
Dim Approved As String
Dim Rejected As String

Set ApprovedSheet = ThisWorkbook.Worksheets("Approved")
Set ImportedSheet = ThisWorkbook.Worksheets("Imported")
Set RejectedSheet = ThisWorkbook.Worksheets("Rejected")

'//////// My ApprovedWorksheet has an extra column, so this add's the extra column prior to copying
ImportedSheet.Columns(6).Insert

'//////// This will be for my counter
iRowStart = ImportedSheet.Rows(2) ' This should Row 2 (first row has headers)
iRowEnd = Cells(Rows.Count, "A").End(xlUp).Row ' Find the last row of data
iCol = ImportedSheet.Columns(5) 'Check Status of "approved" or "rejected"

For iRow = iRowStart To iRowEnd

If ImportedSheet.Cells(iRow, iCol).Value = Approved Then
'/// move to appropriate sheet
iNextRow = ApprovedSheet.Cells(ApprovedSheet.Rows.Count, 1).End(xlUp).Row + 1
ApprovedSheet.Cells(iNextRow, 1).Resize(1, iCol).Value = ImportedSheet.Cells(iRow, 1).Resize(1, iCol).Value
Else
iNextRow = RejectedSheet.Cells(RejectedSheet.Rows.Count, 1).End(xlUp).Row + 1
RejectedSheet.Cells(iNextRow, 1).Resize(1, iCol).Value = ImportedSheet.Cells(iRow, 1).Resize(1, iCol).Value
End If
Next iRow
End Sub

Open in new window

vacation-project-20-jan-13.xls
0
Comment
Question by:bvanscoy678
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 7
  • 5
13 Comments
 
LVL 24

Assisted Solution

by:Steve
Steve earned 1336 total points
ID: 38802238
just change it to 2
iRowStart = 2

Open in new window


column too should be:
iCol = 5

Open in new window


I would also suggest dimming row and column numbers as long rather than integer.
0
 
LVL 93

Assisted Solution

by:Patrick Matthews
Patrick Matthews earned 664 total points
ID: 38802248
iRowStart is declared as an Integer, but ImportedSheet.Rows(2) returns a Range object.

This should work:

iRowStart = ImportedSheet.Rows(2).Row ' This should Row 2 (first row has headers)

Open in new window


Also: never use Integer for a row counter.  Once you hit Row 32,768 you will get an overflow.  Just use Long.
0
 
LVL 24

Accepted Solution

by:
Steve earned 1336 total points
ID: 38802260
Also:

Dim Approved As String
Dim Rejected As String

Open in new window


this will not actually have anything in it.. so "fill" the variable after dimming:
Dim Approved As String: Approved = "Approved"
Dim Rejected As String: Rejected = "Rejected"

Open in new window



it may be easier to drop the variable all together and use:
If ImportedSheet.Cells(iRow, iCol).Value = "Approved" Then

Open in new window

0
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 

Author Comment

by:bvanscoy678
ID: 38802300
working through them now. thanks
0
 

Author Comment

by:bvanscoy678
ID: 38802343
Barman,

This code worked.  I took the suggestion of getting rid of the variable to make it a little less confusing.

I am looking at matthewpatricks suggestion now.

thanks

Sub SortApprovedRejected()

Dim ApprovedSheet As Worksheet
Dim ImportedSheet As Worksheet
Dim RejectedSheet As Worksheet
Dim iCol As Long, iRow As Long, iRowStart As Long, iRowEnd As Long, iNextRow As Long
Dim UniqueIDApprovedWorksheetRng As Range
Dim UniqueIDRejetectedWorksheetRng As Range
 
Set ApprovedSheet = ThisWorkbook.Worksheets("Approved")
Set ImportedSheet = ThisWorkbook.Worksheets("Imported")
Set RejectedSheet = ThisWorkbook.Worksheets("Rejected")

'//////// My ApprovedWorksheet has an extra column, so this add's the extra column prior to copying
ImportedSheet.Columns(6).Insert

'//////// This will be for my counter
iRowStart = 2
iRowEnd = Cells(Rows.Count, "A").End(xlUp).Row ' Find the last row of data
iCol = 8


For iRow = iRowStart To iRowEnd
    
    If ImportedSheet.Cells(iRow, iCol).Value = "Approved" Then
        '/// move to appropriate sheet
        iNextRow = ApprovedSheet.Cells(ApprovedSheet.Rows.Count, 1).End(xlUp).Row + 1
        ApprovedSheet.Cells(iNextRow, 1).Resize(1, iCol).Value = ImportedSheet.Cells(iRow, 1).Resize(1, iCol).Value
    Else
        iNextRow = RejectedSheet.Cells(RejectedSheet.Rows.Count, 1).End(xlUp).Row + 1
        RejectedSheet.Cells(iNextRow, 1).Resize(1, iCol).Value = ImportedSheet.Cells(iRow, 1).Resize(1, iCol).Value
    End If
Next iRow
End Sub

Open in new window

0
 
LVL 24

Expert Comment

by:Steve
ID: 38802356
The suggestion to use
iRowStart = ImportedSheet.Rows(2).Row
is the same as using
iRowStart = 2
There is in reality no difference, I would tend to plump for simplicity.
0
 

Author Comment

by:bvanscoy678
ID: 38802374
I replaced the iRowStart and got an error with:

iCol = ImportedSheet.Columns(8) 'Check Status of "approved" or "rejected"

'//////// This will be for my counter
iRowStart = ImportedSheet.Rows(2).Row  ' This should Row 2 (first row has headers)
iRowEnd = Cells(Rows.Count, "A").End(xlUp).Row ' Find the last row of data
iCol = ImportedSheet.Columns(8) 'Check Status of "approved" or "rejected"
0
 
LVL 24

Expert Comment

by:Steve
ID: 38802385
If you read my first answer you will see I did mention that too.
iCol = ImportedSheet.Columns(8)

Open in new window

should be
iCol = 8

Open in new window

0
 

Author Comment

by:bvanscoy678
ID: 38802398
After working with both of the ideas,  I think i will use The Barman's  because it makes sense to me as I step through it.

Thank you both for the help. I did not realize I was returning a range object, so I'll need to read up on that before I move on.  I have one more step for the loop which is to check for duplicate values (the A column is a unique identifier) prior to copying the row.

I am going to step through this several more times and watch the variables so I make sure I completely get it.

Again, Thanks!
Brent
0
 

Author Closing Comment

by:bvanscoy678
ID: 38802403
Thank you for the help!
0
 
LVL 24

Expert Comment

by:Steve
ID: 38802458
If planning to work with unique values... please take the time to read MathewsPatrics most excellent article on Dictionaries.

It is really handy for what you are doing.
It may be a tad tough at first, but I assure you it is a great article and a realy useful part of VBA.

If you would like to raise another question on how to apply a Dictionary to your code method for unique values, I would be happy to write the code for you to learn from.
0
 

Author Comment

by:bvanscoy678
ID: 38802465
I am reading it now. Thanks  for the link.
0
 

Author Comment

by:bvanscoy678
ID: 38802525
I am not sure it would be appropriate to comment on the article here, but thanks for the link. If I read it correctly, I would set up a dictionary, have my unique values stored into an array, then check my new values against the array, looking for duplicate values.

I'll comment on Patrick's post page.

thanks,
Brent
0

Featured Post

VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

How to get Spreadsheet Compare 2016 working with the 64 bit version of Office 2016
After seeing numerous questions for Dynamic Data Validation I notice that most have used Visual Basic to solve the problem. This suggestion is purely formula based and can be used in multiple rows.
The viewer will learn how to use a discrete random variable to simulate the return on an investment over a period of years, create a Monte Carlo simulation using the discrete random variable, and create a graph to represent the possible returns over…
Finds all prime numbers in a range requested and places them in a public primes() array. I've demostrated a template size of 30 (2 * 3 * 5) but larger templates can be built such 210  (2 * 3 * 5 * 7) or 2310  (2 * 3 * 5 * 7 * 11). The larger templa…

604 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question