1

it's my first post. Could you give me some tips which code is better and why? Mainly those two codes do the same, but using other methods. Cheers

FIRST CODE

Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Age.Click
    Dim age As Integer
    Dim boxinput As String

    boxinput = InputBox("Please input your age", "Age input", "28")

    Try
        age = boxinput


    Catch ex As Exception
        If boxinput = "" Then
            MessageBox.Show("You clicked cancel!", "ERROR", MessageBoxButtons.OK, MessageBoxIcon.Error)

            Exit Sub
        Else
            MessageBox.Show("Correct your age!" & Environment.NewLine & "Input number only!")
        End If

    End Try

End Sub

SECOND CODE

Private Sub Age2_Click(sender As Object, e As EventArgs) Handles Age2.Click
    Dim age As Integer
    Dim boxinput As String

    boxinput = InputBox("Please input your age", "Age input", "28")

    If boxinput = "" Then
        MessageBox.Show("You clicked cancel!", "ERROR", MessageBoxButtons.OK, MessageBoxIcon.Error)
        Exit Sub
    ElseIf IsNumeric(boxinput) Then
        age = CInt(boxinput)
    Else
        MessageBox.Show("Correct your age!" & Environment.NewLine & "Input number only!")
    End If

End Sub
WuWy
  • 11
  • 1
  • 3
    For a start, make sure that Visual Studio is using [`Option Strict On`](https://stackoverflow.com/a/29985039/1115360). Microsoft keep "forgetting" to make it the default for new projects. Then it can automatically show you things that are wrong in the first example, and prevent such errors in new projects for you. – Andrew Morton Apr 17 '20 at 15:01
  • Yeah you're right. I have forgotten about this option. It indicates me that 2nd CODE is more preferable. Thanks! – WuWy Apr 19 '20 at 16:55

2 Answers2

2

There is also Integer.TryParse (and if using other numerics there is TryParse for them too). Also, as pointed out in the comments you should always use meaningful names for controls and the same goes for form names.

Form name should reflect what the form is for and similarly the button name should reflect the action e.g. btnGetPersonAgeButton, GetPersonAgeButton etc.

Consider the following

Public Class Form1
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        Dim age As Integer
        Dim boxinput As String

        boxinput = InputBox("Please input your age", "Age input", "28")

        If Integer.TryParse(boxinput, age) Then
            MessageBox.Show($"Age is {age}")
        Else
            MessageBox.Show($"'{boxinput}' must be a number")
        End If
    End Sub
End Class
Karen Payne
  • 4,341
  • 2
  • 14
  • 31
  • 1
    I think I'd have also picked him up on form and button naming, and declaring variables at the top of the method rather than near point of use :) – Caius Jard Apr 17 '20 at 20:30
  • @CaiusJard - good point, I do this when answering questions on Microsoft forums and sometimes it sticks but not always. – Karen Payne Apr 17 '20 at 20:33
  • I don't think this answers the question which is "Which is better and why?" The other answer does answer the question. – Mary Apr 18 '20 at 17:59
  • TryParse truly looks more decent here. Thanks! – WuWy Apr 19 '20 at 16:57
1

The second option is preferable.

Explanation is copied from here:

Throwing exceptions is expensive. Do not use exceptions to control application flow. If you can reasonably expect a sequence of events to happen in the normal course of running code, you probably should not throw any exceptions in that scenario.

InbetweenWeekends
  • 1,405
  • 3
  • 23
  • 28