Give your code a good name - not test
- it doesn't tell me what it's doing.
At the same time, it's implicitly public
so you might as well put that and always turn on Option Explicit
. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.
You can use Do Until isEmpty
but why not find the last cell and use a For
loop? There is a standard way to find lastRow and lastColumn. That post explains why.
Technically you could eliminate If Cells(i, 2) <> vbNullString
because the lastRow
should determine that, but so would your Do Until isEmpty
so it's already repetitive. You can also avoid assigning i
an initial value and get rid of your messy (for VBA) iteration.
I would also eliminate the if
block, personally.
Option Explicit
Public Sub GetPhonetic()
Dim lastRow As Long
lastRow = Cells(Rows.Count, 2).End(xlUp).Row
Dim i As Long
For i = 1 To lastRow
If Cells(i, 6) <> vbNullString Then Cells(i, 6) = Cells(i, 2).Phonetic.Text
Next i
End Sub
Also eliminate the commented code - if you aren't using it, it's just noise. I also eliminated the extra white space as you don't need it.
Re:
I am also wondering if there is a way to set the starting point to be
Column B (therefore, Cells(i,1)=B1) and possibility replace Cells(i,2)
by dim as something(long?)?
I'm not sure what you mean here, you are already dim i as long
and using that to determine your starting point; there's no need for it to be more complicated than cells(i,y)
.