locked
refactor code RRS feed

  • Question

  • hi guys, in what ways can i refactor this code? 

    [code]

      Private Sub btnNew_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnNew.Click
            If btnNew.Text = "New" Then
                btnNew.Text = "Save"
                enable()
            ElseIf btnNew.Text = "Save" Then
                If txtDiagnosis.Text = String.Empty Then
                    MessageBox.Show("Error encountered! No diagnosis.", "", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
                    Exit Sub
                ElseIf pID = String.Empty Then
                    MessageBox.Show("Error encountered! No patient information.", "", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
                    Exit Sub
                Else
                    If MessageBox.Show("Save information?", "", MessageBoxButtons.YesNo, MessageBoxIcon.Question) = Windows.Forms.DialogResult.Yes Then
                        Try
                            Using dbconn As New SqlConnection(conn)
                                dbconn.Open()
                                Dim dbadd As New SqlCommand("insert into tblcheckups(checkdate, patientid, employeeid, diagnosis, comments, prescription) values(@checkdate, @patientid, @empID, @diagnosis, @comments, @prescription)")
                                With dbadd
                                    .Parameters.AddWithValue("@checkdate", Now.ToShortDateString.Trim)
                                    .Parameters.AddWithValue("@patientid", pID.Trim)
                                    .Parameters.AddWithValue("@empID", emp.Trim)
                                    .Parameters.AddWithValue("@diagnosis", txtDiagnosis.Text.Trim.Replace("'", "''"))
                                    .Parameters.AddWithValue("@comments", txtComment.Text.Trim.Replace("'", "''"))
                                    .Parameters.AddWithValue("@prescription", txtPrescribe.Text.Trim.Replace("'", "''"))
                                    .Connection = dbconn
                                    .ExecuteNonQuery()
                                End With
                                dbconn.Close()
                            End Using
                            MessageBox.Show("Information saved!", "", MessageBoxButtons.OK, MessageBoxIcon.Information)
                            disable()
                            getNames()
                            clear()
                            btnNew.Text = "New"
                        Catch ex As Exception
                            Beep()
                            MessageBox.Show(String.Format("Error encountered! {0}. Source: {1}", ex.Message, ex.Source), "", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
                        End Try
                    End If
                End If
            End If
        End Sub

    [/code]


    programming noob

    Wednesday, January 2, 2013 4:14 AM

Answers

  • Keith,

    Try to forget a little bit your javascript or VBS skills and start to use more possibilities the managed .Net program languages has.

    Your program is a clear sample where the ElseIf can be replaced by the Select Case.

    Avoiding the ElseIf is something that refactors your program while you are busy.

    I also never use the with anymore in the way you do. The with gives me on those places few advantages and if programs grow make them only less readable.

    If you are using the Using then do it consequent, you can nest them so you can nest the Command inside the Connection Using clause.

    Just 3 I think enough for this time.

    Be aware that also a Select case can be nested if needed.

    Refactoring while busy has an easy way. "Think that somebody else should read your program."

    Your program looks if you think that if you use less characters that your program is then smaller and runs quicker, that is not the case. The words are just placeholders for binary addresses and used to make it more easily to read by humans.


    Success
    Cor


    • Edited by Cor Ligthert Wednesday, January 2, 2013 10:22 AM
    • Marked as answer by Youen Zen Wednesday, January 16, 2013 9:48 AM
    Wednesday, January 2, 2013 10:19 AM

All replies

  • Load your project in SharpDevelop and click Refactor.
    Wednesday, January 2, 2013 6:32 AM
  • Keith,

    Try to forget a little bit your javascript or VBS skills and start to use more possibilities the managed .Net program languages has.

    Your program is a clear sample where the ElseIf can be replaced by the Select Case.

    Avoiding the ElseIf is something that refactors your program while you are busy.

    I also never use the with anymore in the way you do. The with gives me on those places few advantages and if programs grow make them only less readable.

    If you are using the Using then do it consequent, you can nest them so you can nest the Command inside the Connection Using clause.

    Just 3 I think enough for this time.

    Be aware that also a Select case can be nested if needed.

    Refactoring while busy has an easy way. "Think that somebody else should read your program."

    Your program looks if you think that if you use less characters that your program is then smaller and runs quicker, that is not the case. The words are just placeholders for binary addresses and used to make it more easily to read by humans.


    Success
    Cor


    • Edited by Cor Ligthert Wednesday, January 2, 2013 10:22 AM
    • Marked as answer by Youen Zen Wednesday, January 16, 2013 9:48 AM
    Wednesday, January 2, 2013 10:19 AM