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

Detailed Last Row Finder

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

Problem

For a project I created a more detailed last row finder which looks at more than one column.

Could you provide any feed back on the code below.

Function lColCount(ws As Worksheet, Optional iWhichRow As Integer = 1) As Long
Dim sMaxCol As String
sMaxCol = Cells(iWhichRow, ws.Columns.Count).Address
lColCount = ws.Range(sMaxCol).End(xlToLeft).Column
Function`

Function lRowCount(ws As Worksheet, Optional iWhichCol As Integer = 1) As Long
Dim sMaxRow As String
sMaxRow = Cells(ws.Rows.Count, iWhichCol).Address
lRowCount = ws.Range(sMaxRow).End(xlUp).Row
End Function

Function Col_Letter(lngCol As Long) As String
Dim vArr
vArr = Split(Cells(1, lngCol).Address(True, False), "$")
Col_Letter = vArr(0)
End Function`

Sub findLast()
Dim LC As Long
Dim LR As Long
Dim LCa As String
Dim ws As Worksheet
Dim i As Integer
Dim arr() As Variant
'Assigns ws = sheet1
Set ws = Sheet1
'Sets LC = to the last column number of default row 1
LC = lColCount(ws)
'I counter = 1
i = 1
'Defines array as 1 to the last column number
ReDim arr(1 To LC)
'Loops untill I = Last column counter + 1
Do While i <> LC + 1
'Sets Lr to last row of the looping column i
LR = lRowCount(ws, i)
'Sets last row to arr(I) value loop
arr(i) = LR
'Adds 1 to I ready for next loop
i = i + 1
Loop
'Finds maximum value in array and uses that as last row
LR = Application.WorksheetFunction.Max(arr())
'Use to check the array value in the Immediate window
Dim Res As Variant
For Each Res In arr()
Debug.Print Res
Next Res
'Col_letter function finds the letter of the column number
LCa = Col_Letter(LC)
'copies range A1 sheet1 and all data to sheet2
ws.Range("A1:" & LCa & LR).Copy Sheet2.Range("A1")
End Sub

Solution

You end two functions like this -

End Function`


Which might be a typo, but it causes errors.

Readability

Indentation

Indenting your code makes it a lot easier to follow and maintain

Sub findLast2()
    '
    Dim i As Integer
    Dim arr() As Variant
    Set ws = Sheet1
    LC = lColCount(ws)
    i = 1
    ReDim arr(1 To LC)
    Do While i <> LC + 1
        LR = lRowCount(ws, i)
        arr(i) = LR
        i = i + 1
    Loop
    LR = Application.WorksheetFunction.Max(arr())
    Dim Res As Variant
    For Each Res In arr()
        Debug.Print Res
    Next Res
    '
End Sub


Comments

"code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.

Naming

Variable names - give your variables meaningful names.

What is LC? lastColumn. LR? lastRow What is vArr() an array of?

Additionally, Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.
Structure

If possible, you should pass argument ByVal rather than ByRef.

You also declare all of your subs and functions as Public by default, so call them public or private to ensure it's clear

Public Sub FindLastRow()
Private Function ReturnColumnLetter(ByVal columnNumber As Long) As String


and so on.

Code Snippets

End Function`
Sub findLast2()
    '<snip>
    Dim i As Integer
    Dim arr() As Variant
    Set ws = Sheet1
    LC = lColCount(ws)
    i = 1
    ReDim arr(1 To LC)
    Do While i <> LC + 1
        LR = lRowCount(ws, i)
        arr(i) = LR
        i = i + 1
    Loop
    LR = Application.WorksheetFunction.Max(arr())
    Dim Res As Variant
    For Each Res In arr()
        Debug.Print Res
    Next Res
    '<snip>
End Sub
Public Sub FindLastRow()
Private Function ReturnColumnLetter(ByVal columnNumber As Long) As String

Context

StackExchange Code Review Q#153126, answer score: 4

Revisions (0)

No revisions yet.