-2

Looking for the best way to write the following code. I am currently struggling to make my code as simple and neat as possible. The code effectively takes a range and returns back the range which is non-empty.

Option Explicit

Sub ReturnNonEmptyRange()

    Dim testBool As Boolean
    Dim i As Long

    testBool = True

    For i = 2 To 10000:
        If Range("G" & i) = "" Then
            i = i - 1
            testBool = False
        End If
        If testBool = False Then
            Exit For
        End If
    Next i

    MsgBox ("The range is G2:K" & i)

End Sub
Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
SamHarper
  • 105
  • 3
  • 23
  • Why not use [`Worksheet.UsedRange.Rows.Count`](https://learn.microsoft.com/en-us/office/vba/api/excel.worksheet.usedrange) – Dean Oct 22 '19 at 07:59
  • 2
    This is best suited for [Code Review Stack Exchange](https://codereview.stackexchange.com/). Also do not hardcode the values or use `UsedRange` to find the last row. You may want to see [THIS](https://stackoverflow.com/questions/11169445/error-in-finding-last-used-cell-in-excel-with-vba/11169920#11169920) – Siddharth Rout Oct 22 '19 at 08:04
  • 1
    Also `If Range("G" & i) = "" Then` can be written as `If Len(Trim(Range("G" & i).Value)) = 0 Then`. and you do not need `testBool` variable. You can directly exit the loop in the first `IF` condition. – Siddharth Rout Oct 22 '19 at 08:07
  • 1
    Question is: Does the OP want to find the last used row in column G or just the used range from G2 to the first empty row in column G? In the former case he could use `Range("G2").End(xlDown)`, in the latter case he can follow the link provided by Siddharth Rout. – Storax Oct 22 '19 at 08:07
  • 1
    @Storax: To loop through the range, it is advisable to find the last row instead of hardcoing the value to `10000` :) – Siddharth Rout Oct 22 '19 at 08:08
  • @Siddhart Rout: Yes, hardcoding 10000 is strange. IMO there is no need to loop through the range. – Storax Oct 22 '19 at 08:09
  • The range `G2:K10000` could also be an array. In which case finding the last one would not work. – SamHarper Oct 22 '19 at 08:11
  • https://stackoverflow.com/questions/14957994/select-first-empty-cell-in-column-f-starting-from-row-1-without-using-offset – Siddharth Rout Oct 22 '19 at 08:12
  • 1
    You say you are working wiht an array but are looping through a range? If you are working with an array then use `Ubound(MyArray)` to get the upper bound of the array. – Siddharth Rout Oct 22 '19 at 08:14
  • According to your code you work with a range. – Storax Oct 22 '19 at 08:14
  • But the range could also be part of an Array? – SamHarper Oct 22 '19 at 08:15
  • 1
    Then please provide the code where you do that. By the code posted above we are talking about a range. – Storax Oct 22 '19 at 08:16

4 Answers4

1

Below is some sample code you can try.

The function LastUsedRow is not used, but I'm providing since it can be useful. This will return the last used row in your worksheet.

Using "Range" like you did above will assume you want to use active sheet. I always like to specify a workbook and a sheet so there is no ambiguity.

Sub Test()

    ' Start at row 1 and and stop when first blank cell found

    Dim wks As Worksheet: Set wks = ThisWorkbook.Worksheets("Sheet1")
    Dim row As Long

    ' Option 1: using column numbers
    row = 1
    Dim col As Long: col = 7    ' G
    Do Until wks.Cells(row + 1, col).Value = ""
        row = row + 1
    Loop
    MsgBox ("Last used row (in column): " & row)    ' assumes something in row 1

    ' Option 2: using column letters
    row = 1
    Dim colLetter As String: colLetter = "G"
    Do Until wks.Range(colLetter & row + 1).Value = ""
        row = row + 1
    Loop
    MsgBox ("Last used row (in column): " & row)    ' assumes something in row 1

End Sub

Public Function LastUsedRow(wks As Worksheet) As Long
    Dim rng As Range: Set rng = wks.UsedRange   ' Excel will recalc used range
    LastUsedRow = rng.row + rng.Rows.Count - 1
End Function
Ernie Thomason
  • 1,579
  • 17
  • 20
0

I think your method only works if your none-empty range is consecutive. Suppose G2:G10 is non-empty, G11 is empty and G12:G20 is non-empty. Your code would come to i=11 and return G2:K10 as the non-empty range.

A more reliable, and quicker way to find the last non-empty cell (before row 1000) would be this:

range("G1000").End(xlUp).row

This will give you the first non-empty row in column G above row 1000. If row 1000 is non-empty however, it would search upwards for the last non-empty row. so you might want to change it to:

Range("G" & Rows.Count).End(xlUp).Row

This will find the last non-empty row, starting from the bottom of the worksheet.

Beek
  • 376
  • 1
  • 10
0

How about combining the loop's exit conditions all into the loop control header.

I also would explicitly access the range()'s value to be more clear in the code and check the string length to be zero.

Option Explicit

Sub ReturnNonEmptyRange()

    Dim testBool As Boolean
    Dim i As Long

    testBool = True

    i = 2
    While (i < 10000) And (Len(Range("G" & i).Value) <> 0)
        i = i + 1
    Wend

    MsgBox ("The range is G2:K" & i)

End Sub
Marco
  • 11
-1

In the case this was an Array, one could not use Range("G" & Rows.Count).End(xlUp).Row. I believe @Siddharth provided a good solution. The downside being it will stop at a non- empty row.

Sub ReturnNonEmptyRange()

    Dim i As Long

    For i = 2 To 10000:
        If Len(Trim(Range("G" & i).Value)) = 0 Then Exit For
    Next i

    MsgBox ("The range is G2:K" & i - 1)

End Sub

SamHarper
  • 105
  • 3
  • 23