ERP-system with UserForm that transfers data to next available row in an archive spreadsheet

Question

I made this huge code (Edit: huge for such a simple task) for transferring data and adding an ID number to every new row of data. The only functionality flaw I see is that it creates new ID numbers if you transfer blank cells. I was wondering if anyone can help me improve this code? I'm very bad at loops so if there is any possibilities for loops that I overlooked please tell me.

Edit: also had to add another worksheet for temporarily storing the data, so I could copy it as a range, since it is first saved as a value.

Private Sub CommandButton2_Click()

'Variabler for inputs
shipFrom = TextBox1.Text
shipTo = TextBox2.Text
shipDate = TextBox3.Text
NP = TextBox4.Text
desc = TextBox10.Text
gramEx = TextBox6.Text
tareWeight = TextBox7.Text
weight = TextBox8.Text
dims = TextBox9.Text

Sheets("test").Range("b2").Value = shipFrom
Sheets("test").Range("c2").Value = shipTo
Sheets("test").Range("d2").Value = shipDate
Sheets("test").Range("e2").Value = NP
Sheets("test").Range("f2").Value = desc
Sheets("test").Range("g2").Value = gramEx
Sheets("test").Range("h2").Value = tareWeight
Sheets("test").Range("i2").Value = weight
Sheets("test").Range("j2").Value = dims



Dim sf As Range
Set sf = Sheets("test").Range("b2")

Dim st As Range
Set st = Sheets("test").Range("c2")

Dim d As Range
Set d = Sheets("test").Range("d2")

Dim l As Range
Set l = Sheets("test").Range("e2")

Dim i As Range
Set i = Sheets("test").Range("f2")

Dim g As Range
Set g = Sheets("test").Range("g2")

Dim t As Range
Set t = Sheets("test").Range("h2")

Dim w As Range
Set w = Sheets("test").Range("i2")

Dim di As Range
Set di = Sheets("test").Range("j2")



Dim nextRowB As Range
Set nextRowB = Sheets("Arkiv").Range("B" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowC As Range
Set nextRowC = Sheets("Arkiv").Range("C" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowD As Range
Set nextRowD = Sheets("Arkiv").Range("D" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowE As Range
Set nextRowE = Sheets("Arkiv").Range("E" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowF As Range
Set nextRowF = Sheets("Arkiv").Range("F" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowG As Range
Set nextRowG = Sheets("Arkiv").Range("G" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowH As Range
Set nextRowH = Sheets("Arkiv").Range("H" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowI As Range
Set nextRowI = Sheets("Arkiv").Range("I" & Rows.Count).End(xlUp).Offset(1, 0)

Dim nextRowJ As Range
Set nextRowJ = Sheets("Arkiv").Range("J" & Rows.Count).End(xlUp).Offset(1, 0)



sf.Copy nextRowB
st.Copy nextRowC
d.Copy nextRowD
l.Copy nextRowE
i.Copy nextRowF
g.Copy nextRowG
t.Copy nextRowH
w.Copy nextRowI
di.Copy nextRowJ



Dim ID As Range
Set ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0)



If IsEmpty(Sheets("Arkiv").Range("B2").Value) Then


    MsgBox "Please fill in the <Ship From> box"


Else

    'Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0) = "1"
    Sheets("Arkiv").Range("A2").Value = "1"


End If


If IsEmpty(Sheets("Arkiv").Range("C2").Value) Then


Else


    ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value + 1


End If

Sheets("Arkiv").Activate
MsgBox "The ID number for this data is: " & Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value




End Sub

Answer 1

Ouch. I say this in a kind way - the code is painful to read.

1. Use Option Explicit at the top of every module - always!
2. Properly indent your code - it makes it easier to read, easier to pick up inconsistent logic and easier to maintain.

Arrays, and Excel's ability to convert values of ranges to arrays is your friend in this instance. No need for loops.

Private Sub CommandButton2_Click()
Dim inputs as Variant 'Variabler for inputs
    inputs = Array(TextBox1.Text, TextBox2.Text, TextBox3.Text, TextBox4.Text, _
                   TextBox10.Text, TextBox6.Text, TextBox7.Text, TextBox8.Text, _
                   TextBox9.Text)
    ' order is important for you here.
    ' shipFrom, shipTo, shipDate, NP, desc, gramEx, tareWeight, weight, dims

Dim nextRowB As Range
    Set nextRowB = Sheets("Arkiv").Range("B" & Rows.Count & ":J"& Rows.Count).End(xlUp).Offset(1, 0)
    nextRowB.Value = inputs

    ' Now that we put everything into the array, the individual ranges and the ".copy" is not longer required
    ' sf.Copy nextRowB etc <-- not needed, see line of code above!

Dim ID As Range
    Set ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0)
    If IsEmpty(Sheets("Arkiv").Range("B2").Value) Then '<-- This is your logic, but not sure if this is really what you want.
    ' Is it really only row 2 that you check?
        MsgBox "Please fill in the <Ship From> box"
    Else
        'Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Offset(1, 0) = "1"
        Sheets("Arkiv").Range("A2").Value = "1"
    End If
    If Not IsEmpty(Sheets("Arkiv").Range("C2").Value) Then ' added "Not" here, why have an empty code block?
    'Else <-- why have an empty code block?
        ID = Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value + 1
    End If
    Sheets("Arkiv").Activate
    MsgBox "The ID number for this data is: " & Sheets("Arkiv").Range("A" & Rows.Count).End(xlUp).Value
End Sub

Two other things to look at to make this more readable:

• Consider assigning "Sheets("Arkiv")" to a variable (e.g. ws) and using that throughout.
• Consider using With Sheets("Archiv") block.

And finally, ensure that all references to range objects and methods are fully qualified. This includes the Rows elements within the Range(...) code.

Edit (From Stack Overflow @AhmedAU, alternate way of inserting the values - this has been tested and works according to OP):

Dim inputs As Variant
inputs = Array(TextBox1.Text, TextBox2.Text, TextBox3.Text, TextBox4.Text, TextBox10.Text, TextBox6.Text, TextBox7.Text, TextBox8.Text, TextBox9.Text)
Dim nextRowB As Range
Set nextRowB = Sheets("Arkiv").Range("B" & Rows.Count).End(xlUp).Offset(1, 0)
nextRowB.Resize(1, UBound(inputs) + 1).Value = inputs