I need help cleaning up this code.

[khodjo]

Just to preface this by saying i am a novice at VBA. Started about 3 weeks ago.
Anyhow, what I am I am trying to do with this code is this.

Create Header - creates the header.

Add Item
This is supposed to open a dialog box where I enter the item number from a list named as ITEM_NUMBER and the quantity.

The entry is then placed on the sheet and a vlookup is invoked to bring up the description from the list.

The dialog is then cleared to receive the next entry which is then placed on the next row. and so on.

the header code works.

the Vlookup does not. Please help...Beinga beginner, all snarky comments are welcome. thanks.

The file is attached.

[lucas]

Moved to the excel help forum.

[khodjo]

Thanks VBAX. My apologies.

[mdmackillop]

Try this, using a combobox for easier data entry.

[Aussiebear]

Just a couple of quick comments

1. When writing code try to use the "Option Explicit" function. This forces you to define any variables. Great habit to get into particularly when learning to code.

2. Minimise the use of ".Select" While the Macro Recorder does this, most programmers will tell you that it simply slows the system down. In your code example you used the following:

[VBA]
Range("A2:F2").Select
With Selection Range("A2:F2").Font
.Themecolor = 5
.Size = 12
.Name = "Calabri"
.Bold = True
[/VBA]

It can be more easily written as

[VBA]With Range("A2:F2").Font
.ThemeColor = 5
.Size = 12
.Name = "Calibri"
.Bold = True
End With
[/VBA]

3. I don't actually see any code for the lookup function in your example

[mdmackillop]

Other ways to check the number is correct

[Paul_Hossler]

I always like to use a worksheet just to make sure that the sheet I think is active, is the same sheet Excel thinks is active

[vba]
Worksheets("Estimate of Quantities").Range("A1:F1").Merge
[/vba]


Also hard coded "will never be more than 100" rows seem to always grow

[vba]
Range("A3:F100").ClearContents
[/vba]

What I like to do is let Excel do all the work

[vba]
Dim rData As Range

Set rData = Worksheets("Estimate of Quantities").Cells(1, 1).CurrentRegion

With rData
.Cells(3, 1).Resize(.Rows.Count - 2, .Columns.Count).ClearContents
End With
[/vba]


My opinions / personal style

Paul

[khodjo]

Thanks for all the responses so far. This is the code I am using for the OK button. The named range is "catalog" and the sheet it was named from is Item Catalog.
The vlookup line is supposed to take what was transferred from the dialog box into the first column, look it up in the named range Catalog and return that to the second column on the same line.

Here is what I have so far.:
[VBA]Sub cmdOK_Click()
Dim NextRow As Long
Dim inumber As Long
Dim ws As Worksheet

' Activate the worksheet
Sheets("Estimate of Quantities").Activate

'Determine the next empty row
NextRow = WorksheetFunction.CountA(Range("A:A")) + 1

'Transfer New Item Number and Quantity

Cells(NextRow, 1) = txtItemNo.Text
Cells(NextRow, 3) = txtQty.Text

'Use vlookup to get description from named range

inumber = Cells(NextRow, 1)
Set ws = Sheets("ITEM CATALOG")
Result = Application.VLookup(inumber, ws.Range("ITEM_NUMBER"), 3, False)

If IsError(Result) Then
MsgBox "No Match Found, Enter a Correct Number"
Else
Cells(NextRow, 2) = Result
End If


'Clear contents for next entry
txtItemNo.Text = ""
txtQty.Text = ""
txtItemNo.SetFocus

End Sub[/VBA]

[Simon Lloyd]

Cross posted and answered here http://www.mrexcel.com/forum/showthread.php?t=436439

[Simon Lloyd]

khodjo, please take a minute to read the forum rules and the link in my signature with regards cross posting!