asp classic - refactor advice

Question

    d("rma_date") = request.form("FormDate")
d("rma_phone") = request.form("FormPhone")
d("rma_orderno") = request.form("FormOrderNumber")
d("rma_email") = request.form("FormEmail")
d("rma_partno") = request.form("partno")
d("rma_comment") = request.form("comment")
d("rma_note") = request.form("note")
d("rma_failtype") = request.form("failtype")

''' from this
dim handleTypeArray
handleTypeArray = Split(request.form("handletype"), ",")
Dim counter  
For counter = 0 To UBound(handleTypeArray)  
  if handleTypeArray(counter) <> " " then
    d("rma_handletype") = handleTypeArray(counter)
  end if 
Next
''' to this

d("rma_exchanged_partno") = request.form("exchanged_partno")
d("rma_account_number") = request.form("account_number")
d("rma_reg_number") = request.form("reg_number")
d("rma_error_description") = request.form("error_description")
d("rma_vareno") = request.form("varenumer[]")
d("sent") = true
'SoapCheckRMA(d)
  'd("rmano") = SoapCheckRMA(d)
  'test
  d("rmano") = "something else"
r = rendertemplate("templates\rma_new.done.html", d)
HandleRMANew=Translate(r, session("language"), "rma_new")

Should I refactor the code from the block from here to to here. Because I find myself too focusing in developing and create beautiful code. It takes too much time, since I should code more feature instead.

But I want a readable code so I can save bunch of time for me and another developers to work on this later. My goals of writing code is easy to read and maintain later. So what is the strategy in this case :

1. Add a comment to explain what I do here
2. Create a simple helper function to get the form value in one line like others above lines.

Answer 1

Yes, I would put it in a function with a meaningful name. That way, as you are reading through the code above, and you get to that line, you just say, "ok, now go off and do X" rather than trying to figure out what X is. If you need to know what X is, you go read the function.

As for your question on comments, comments used as you would like to use them are a code smell.

Read this post on comments, Or this post if you don't believe me

This:

d("rma_failtype") = request.form("failtype")

''' from this

ProcessRmaHandleType(d, request.form("handletype"))

''' to this

d("rma_exchanged_partno") = request.form("exchanged_partno")

reads a lot easier than the original, and honestly, it wouldn't of taken me any more time to write.