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

Checking for missing values in several columns of a spreadsheet

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

Problem

Basically I had 3 columns of Data

  • Adjudicated Cases



  • Not Adjudicated Cases



  • All Cases



and I wanted to get a list of all the cases that were missing from Column A and Column B, with a little help from @RubberDucky and his really cool Enumerable.cls (Found on GitHub in His VBX Repository) I used the Merge function to first merge Column A and Column B together in an Enumerable and then using the Contains function I checked to see if the Values in the Enumerable were contained in Column C, if they weren't there I added them to a second Enumerable and then printed that one out into another column.

How can I further hone my VBA skills?

Sub FindUnAssigned()
    With Sheets("FY13")
        Dim e As New Enumerable
        Dim lastARow As Integer
        lastARow = .Range("A" & .Rows.Count).End(xlUp).Row
        Dim lastBRow As Integer
        lastBRow = .Range("B" & .Rows.Count).End(xlUp).Row

        Set e.Collection = Range("A3:A" & lastARow)
        e.Merge (Range("B3:B" & lastBRow))

        Dim cell As Range
        Dim columnD As New Enumerable
        Dim lastCRow As Integer
        lastCRow = .Range("C" & .Rows.Count).End(xlUp).Row

        For Each cell In Range("C3:C" & lastCRow)
            If Not (e.Contains(cell.value, True)) Then
                columnD.Add (cell.value)
            End If
        Next cell

        Dim i As Integer
        For i = 1 To columnD.Count
            Dim j As Integer
            j = i + 2
            Range(("D" & j), ("D" & columnD.Count + 2)) = columnD.item(i)
        Next i
    End With
End Sub

Solution


  • I discourage the use of single letter variable names. So, I recommend changing e to something else. I'm drawing a blank though because you can't use Cells in Excel's flavor of vba. That's the object that refers to the sheet's collection of all cells. Maybe cases would work given the context.



  • I like your use of the With statement here. I'm tossing that out there because I know others will disagree with me. With can be abused, but this is not the case with your code.



  • When declaring an iterator variable for a ForEach statement, I like to declare it just outside the loop. You should move Dim cell As Range.



  • j is a useless variable. I would just add two to i instead.



-
Speaking of 2, it's a magic number. I would consider declaring an offset variable.

Dim i As Integer
    Const offset As Integer = 2

    For i = 1 To columnD.Count
        Range(("D" & i + offset), ("D" & columnD.Count + offset)) = columnD.item(i)
    Next i


-
I take back what I said about the With statement. It would be a good use, but you're using Range wrong. For example, this line doesn't do what you think it does.

Range(("D" & i + offset), ("D" & columnD.Count + offset))


You're using the global Range object, which is the same as saying ActiveSheet.Range(). That's not what you want. This can lead to some hard to track down heisenbugs. You want to work on your named sheet. You need to add the dot in order to be working on the named sheet.

.Range(("D" & i + offset), ("D" & columnD.Count + offset))


From here on out I'm at a bit of an unfair advantage because I wrote the Enumerable class that you're using.

-
You don't need the .value when using the useDefaultProperty option. Value is the default property of any Range object.

If Not (e.Contains(cell.value, True)) Then


Can be changed to

If Not (e.Contains(cell, True)) Then


-
The same thing applies to the Add method.

columnD.Add (cell.value)


-
Enumerable uses a magic attribute that gives it a default property as well. So, just like the built in Collection, there's no reason to use the verbose method of getting an item. Item is the default property.

columnD.item(i)


Becomes

columnD(i)

Code Snippets

Dim i As Integer
    Const offset As Integer = 2

    For i = 1 To columnD.Count
        Range(("D" & i + offset), ("D" & columnD.Count + offset)) = columnD.item(i)
    Next i
Range(("D" & i + offset), ("D" & columnD.Count + offset))
.Range(("D" & i + offset), ("D" & columnD.Count + offset))
If Not (e.Contains(cell.value, True)) Then
If Not (e.Contains(cell, True)) Then

Context

StackExchange Code Review Q#65309, answer score: 5

Revisions (0)

No revisions yet.