Filling a row with values

I started with VBA yesterday. Could this code be improved?

Private Sub Worksheet_Change(ByVal target As Range)
    If target.Column = 3 Then
        Call Modul1.AutoFill(target)
    End If
End Sub

Option Explicit

Sub AutoFill(activeCell As Range)
    Dim storedData As Range: Set storedData = GrabStoredData(activeCell.Value)
    If Not storedData Is Nothing Then
        Call FillRowWith(activeCell, storedData)
    End If
End Sub

Private Function GrabStoredData(itemName As String) As Range
    Dim itemCell As Range
    Set itemCell = Sheets("SourceSheet").Range("A:A").Find(what:=itemName, lookat:=xlWhole)

    If Not itemCell Is Nothing Then
        Set GrabStoredData = Sheets("SourceSheet").Range("B" & itemCell.row & ":G" & itemCell.row)
    End If
End Function

Private Sub FillRowWith(activeCell As Range, storedData As Range)
    Dim counter As Integer
    For counter = 1 To 6
        Sheets("TargetSheet").Cells(activeCell.row, activeCell.Column + counter).Value = storedData.Offset(0, counter - 1).Value
    Next counter
End Sub

We have a SourceSheet that works as the "Database". The SourceSheet has 7 columns, with the first one used to identify an item or order. The other 6 columns contain information.

Lets say we put an order in "TargetSheet" which already exists in "SourceSheet". It will find the order in "SourceSheet" and take the information in the 6 columns and replace 6 columns in "TargetSheet" with that information.

This demo should show it better.

:

Don't.

People read code downwards much better than they read across.
Almost all code everywhere operates under the convention "(at most) 1 statement/operation per line". Colons : are small and very easily missed. Just stick subsequent operations on newlines and you'll avoid a lot of confusion/errors down the line.

Call is fully deprecated

It's not needed anymore and your code will run just fine without it. Assuming you name your methods well, dropping Call will also means your code reads better.

You will need to modify your calling syntax slightly though. When you have a DoThing Sub, this is valid syntax:

Call DoThing(Arg1, Arg2)

But this is not:

DoThing(Arg1, Arg2)

Functions get brackets. Subs do not. Like so:

Var = Function(Arg1, Arg2)
DoThing Arg1, Arg2

Don't let renaming break your code

Call Modul1.AutoFill(target)

Sheets("SourceSheet").Range("B" & itemCell.row & ":G" & itemCell.row)

If that module gets renamed, or that sheet gets renamed, your code will break.

Fortunately there are easy solutions.

1: Don't qualify your module references

You shouldn't need to specify which module your subs are in. If you do, this implies wider problems with your code. If your methods are descriptively named and you don't do something horrible like having 2 identically-named methods in different modules, just call your method using its signature.

2: Sheets have Codenames

If you go to the properties window and select a Worksheet, there will be a (name) property. This is the sheet's Codename. By default, new sheets have codenames like Sheet1, Sheet2 etc. A Codename is effectively a global worksheet variable that always refers to that sheet.

So, assuming you give "SourceSheet" a Codename of sourceSheet then your code can simply be:

sourceSheet.Range("B" & itemCell.row & ":G" & itemCell.row)

And now your users can name the sheets whatever they like and your code will still run fine.

Be Explicit with your naming

Your method names are OK, but very, very ambiguous. AutoFill, GrabStoredData, FillRowWith, they all imply a level of generality that is at odds with what they actually do.

As a rule, names should be descriptive, then unambiguous and only then concise. Easy-To-Understand trumps Shorter-Name every time.

GrabStoredData(itemName). Not a clue what that does in particular.

GetRowRangeOfItemData(itemName). I immediately know exactly what that does. It's going to return a Range Object for a given item. This Range Object will be on a single row.

AutoFill(Cell). Fill What? Where? With what?

FillItemDetailsToRightOfCell(Cell). Takes about 0.2 seconds longer to read. Takes much less time to understand. Especially when someone else (or you in 6 months time) is coming back to this code with no memory of the internal workings.

FillRowWith(Range, Cell). If I see that I'm going to assume it, well, fills a row. No Idea what those variables are, particularly.

PasteRangeToRightOfCell(Range, Cell). Again, takes a fraction of a second longer to read, takes no time to understand exactly what's going on.

StackExchange Code Review Q#134072, answer score: 6