HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Loop through range, selecting grouped data?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rangeloopgroupedselectingthroughdata

Problem

I have some info. that looks like this:

Header  Debit   Credit

aaaa    20  0

bbbb    0   60

cccc    30  0

xxxx    10  0
xxxx    0   40
xxxx    40  0
xxxx    0   10

oooo    500 0
oooo    0   52
oooo    0   500

pppp    10  
pppp    0   10


(Note this starts in col. H)

The idea is that I want to go through this range, and where there are "blocks" of data (see the "xxxx" group), check for offsetting amounts.

I was able to put this together, to loop through the groups, but I'm not sure if it's the best way to do so. I'm afraid there will be a time it skips a line which is unintended - any ideas/tips on improving it?

Sub strike_out_Matching_Debit_and_Credits()
Dim debitCol&, credCol&, lastRow&, startRow&, endRow&, i&
Dim rng As Range, cel As Range
Dim myWS As Worksheet

Set myWS = Sheets("Raw - edited")
With myWS
    debitCol = .Rows(1).Find(what:="Debit").column
    credCol = .Rows(1).Find(what:="Credit").column
    lastRow = .Cells(.Rows.Count, credCol).End(xlUp).row

startRow = 2
    For i = 2 To lastRow
        endRow = .Cells(i, debitCol).End(xlDown).row
        Set rng = .Range(.Cells(startRow, debitCol), .Cells(endRow, debitCol))

        rng.Select

        If rng.Cells.Count - WorksheetFunction.CountA(rng) = 0 And rng.Cells.Count > 1 Then
            ' We now have a block to work through
            check_Values rng, rng.Offset(0, 1)
            check_for_Offset_Values rng, rng.Offset(0, 1)
        End If
        startRow = .Cells(endRow, debitCol).End(xlDown).row
    Next i

End With
End Sub


Thanks for any ideas/advice/tips!

Edit: I appreciate the help so far! I've made notes on the naming conventions, etc. Does anyone have any tips for tightening up the actual function of the code loop? That's what I'm mainly interested in.

Solution

Following on your (shared) need of tightening up your code:

Get rid of unused variables

As per your code you don't use neither creditCol nor cel, so get rid of them

Avoid useless variables

You don't need myWS and can simply go

With Worksheets("Raw - edited")


where you also want to use Worksheets instead of Sheets since the former is the collection of sheets with actual data in cells while the latter is the collection that takes "chart" sheets too.

You don't need any lastRow, startRow, endRow and i variable.

Just loop through a "well" selected range using SpecialCells property of Range object and one of your already declared variable of Range type for the task.

Use Areas property of Range object

this will let you loop through the actual "blocks" (as you're naming them) of the parent range, i.e. the groups of contiguos cells that range is made of

Choose proper row counter column

As already pointed out in previous answers, from your data example "Credit " column has blank cells that may prevent you from picking the actual last one, while "Header" column seems to be the best candidate (and of which you already know column index,being it "H")

So choose this latter or loop through all relevant columns and calculate the maximum lastRow between them (make a Function for this task)

Proper use of Find method of Range object

It always "remembers" and uses the last user's choice of some options, were it made from Excel UI or any VBA code.

So you'd better always specify those options and ensure they're those you actually need

Find(What:="Debit", LookIn:= xlValues, LookAt:= xlWhole, MatchCase:= False)


Final result

All what above toghether results in the following code

Sub strike_out_Matching_Debit_and_Credits()
Dim debitCol As Long
Dim block As Range 

With Worksheets("Raw - edited")
    DebitCol = .Rows(1).Find(What:="Debit", LookIn:= xlValues, LookAt:= xlWhole, MatchCase:= False)
    For Each block In .Columns("H").SpecialCells(xlCellTypeConstants, xlTextValues).Areas
        If block.Count > 1 Then
            check_Values block.Offset(, debitCol - 8), block.Offset(, debitCol - 7)
            check_for_Offset_Values block.Offset(, debitCol - 8), block.Offset(, debitCol - 7)
        End If
    Next block 
End Sub


This also assumes that:

-
"Debit" is there for sure in some cell of row 1

-
column "H" has at least one cell with text value

were these assumptions unsafe then you'd add some "entry" checks, possibly wrapped in a specific function returning a boolean to tell your main Sub whether to proceed or exit

Code Snippets

With Worksheets("Raw - edited")
Find(What:="Debit", LookIn:= xlValues, LookAt:= xlWhole, MatchCase:= False)
Sub strike_out_Matching_Debit_and_Credits()
Dim debitCol As Long
Dim block As Range 

With Worksheets("Raw - edited")
    DebitCol = .Rows(1).Find(What:="Debit", LookIn:= xlValues, LookAt:= xlWhole, MatchCase:= False)
    For Each block In .Columns("H").SpecialCells(xlCellTypeConstants, xlTextValues).Areas
        If block.Count > 1 Then
            check_Values block.Offset(, debitCol - 8), block.Offset(, debitCol - 7)
            check_for_Offset_Values block.Offset(, debitCol - 8), block.Offset(, debitCol - 7)
        End If
    Next block 
End Sub

Context

StackExchange Code Review Q#133208, answer score: 3

Revisions (0)

No revisions yet.