4
  1. Created a userform
  2. Added a textBox and a comboBox
  3. Added a submit button
  4. When submit is clicked it adds the data to a spreadsheet

From what I have been told and what I have read this is wrong

ActiveCell.Value = TextBox3.Text 
ActiveCell.Offset(0, 1).Select   
ActiveCell.Value = ComboBox1.Text  
ActiveCell.Offset(1, -1).Select  

This works but I've been told I shouldn't use the .select keyword when possible. I've read that to make my code reusable I should create variables. How would a professional developer write this code, can it be written in less lines and how can I refer to the activecell offset without using select?

Community
  • 1
  • 1
Pete
  • 171
  • 1
  • 5
  • 22
  • 4
    possible duplicate of [How to avoid using Select in Excel VBA macros](http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros) – Alex K. Jan 05 '15 at 16:37
  • It's not wrong! You've recorded a macro and just use it as is; it works for you and, hence, it is OK. It is not really programming but why would you care? Also, fewer lines is not better or worse in itself, just fewer. If it is readable, it is OK. – Gene Skuratovsky Jan 05 '15 at 16:38
  • In order to answer your question, we would need to know: The cell you want your `TextBox3` to dump into, the cell you want `ComboBox1` to dump into (looks like `Offset(0, 1)` from TextBox3 cell) and then what you are trying to do with the `Offset(1, -1)` cell. (Edit: Never mind I think I see what you are trying to do: Add an entry on the next blank line) – Chrismas007 Jan 05 '15 at 16:41

2 Answers2

13

I am assuming you want TextBox3 in column A and ComboBox1 in column B. If you want different columns just change the letter references.

Sub OnClick() 'whatever your current sub is called.

    Dim LastRow As Long, ws As Worksheet

    Set ws = Sheets("Name of Sheet where data is going")

    LastRow = ws.Range("A" & Rows.Count).End(xlUp).Row + 1 'Finds the last blank row

    ws.Range("A" & LastRow).Value = TextBox3.Text 'Adds the TextBox3 into Col A & Last Blank Row
    ws.Range("B" & LastRow).Value = ComboBox1.Text 'Adds the ComboBox1 into Col B & Last Blank Row

End Sub

If you want a method using Offset():

Sub OnClickwithOffset() 'whatever your current sub is called.

    Dim LastRow As Long, ws As Worksheet

    Set ws = Sheets("Name of Sheet where data is going")

    LastRow = ws.Range("A" & Rows.Count).End(xlUp).Row + 1 'Finds the last blank row

    ws.Range("A" & LastRow).Value = TextBox3.Text 'Adds the TextBox3 into Col A & Last Blank Row
    ws.Range("A" & LastRow).Offset(0, 1).Value = ComboBox1.Text 'Adds the ComboBox1 into next cell to the right of TextBox3 data.

End Sub
Chrismas007
  • 6,085
  • 4
  • 24
  • 47
0

The main reason why you want to avoid using ActiveCell is that it may produce unexpected results should the user select another cell while the code is executing.

If your goal is to always write the contents of your controls to the same cells, you may wish to start by defining a variable of type Range and set your offsets relative to that variable.

E.g.:

Dim myCell as Range

Set myCell = ThisWorkbook.Sheets(1).Range("C4")
myCell.Value = TextBox3.Text
myCell.Offset(0, 1).Value = ComboBox1.Text

'[...]
silentsurfer
  • 1,998
  • 2
  • 17
  • 29
  • This doesn't quite accomplish his task of imputing at the last row. – Chrismas007 Jan 05 '15 at 16:45
  • Where does it say his task is to input at the last row? – silentsurfer Jan 05 '15 at 16:48
  • His `ActiveCell.Offset(1, -1).Select` code indicates that is his intention. To input at a specific line, then set the focus on the next line. – Chrismas007 Jan 05 '15 at 16:49
  • 1
    That is your interpretation of his code. Nowhere in the description does it say that data should be appended. And anyway, his question was how to avoid using the "select" keyword. Please read carefully before downvoting. – silentsurfer Jan 05 '15 at 16:51
  • 1
    _"How would a professional developer write this code"_ is the question. Your answer does not in any way address the last line of the user's code. – Chrismas007 Jan 05 '15 at 17:05
  • 1
    Well, your answer does not address the question " how can I refer to the activecell offset without using select". It is a solution to your interpretation of the problem and does not address the conceptual questions raised by the asker. I admit that if the question were how to append data to a worksheet, I would have picked an approach quite similar to yours. But that is not the point in this case and your argumentation certainly does not justify a downvote. – silentsurfer Jan 05 '15 at 17:17