PHP User Warning: fetch_template() calls should be replaced by the vB_Template class. Template name: bbcode_highlight in ..../includes/functions.php on line 4197
VS 2017 [RESOLVED] Better coding?-VBForums
Results 1 to 3 of 3

Thread: [RESOLVED] Better coding?

  1. #1

    Thread Starter
    Addicted Member Mallard8's Avatar
    Join Date
    Feb 2018
    Location
    Wales
    Posts
    171

    Resolved [RESOLVED] Better coding?

    I have a button that the user clicks after they enter a word, to make sure the word is at least 4 letters long I have an If Then statement that hows a MsgBox. When they click OK focus is returned to the form so they can type a letter of the correct length. Below is the code I'm using to achieve this and it all works fine but is it right?
    Code:
            If lblBLetter4.Text = "" Then   'Word must contain at least 4 letters
                MsgBox("Your word must have at least 4 letters", CType(vbOK, MsgBoxStyle), "No Valid Word")
                btnBlue.Enabled = False  'Disable
                Me.Focus()               'Return focus to screen
                btnBlue.Enabled = True   'Enable
                Exit Sub
            End If
    Learning is a never ending subject.

  2. #2
    .NUT jmcilhinney's Avatar
    Join Date
    May 2005
    Location
    Sydney, Australia
    Posts
    102,018

    Re: Better coding?

    There are a number of things that could be improved there.

    If the point is the check that text entered by the user is at least four characters in length, why is it checking the contents of a Label, which doesn't accept user input, and why is it checking only for an empty String, which would not detect input of 1 to 3 characters?

    You really ought to be calling MessageBox.Show in VB.NET, rather than MsgBox, which is a holdover from VB6. If you are going to use MsgBox, it doesn't make sense to convert vbOK to a MsgBoxStyle value. You should be using an actual MsgBoxStyle value, e.g. MsgBoxStyle.OkOnly. If you want to combine multiple styles then you use the Or operator to combine them bitwise. MessageBox.Show doesn't require bitwise combinations because it accepts an argument for each element.

    It doesn't make sense to be disabling a Button and then enabling it again in the same method because the user can't click it while that code is being executed anyway.

    An application developer should be calling Focus. As the documentation says, you should be calling Select.

    It is recommended to use Return rather than Exit Sub.

    If you want to validate a control then it is generally best to handle the Validating event of that control. If the contents fails validation, you simply set e.Cancel to True and the control not lose focus in the first place. You can call the form's ValidateChildren method to raise the Validating event for every control, which ensures even controls that never receive focus will be validated, e.g.
    vb.net Code:
    1. Private Sub TextBox1_Validating(sender As Object, e As CancelEventArgs) Handles TextBox1.Validating
    2.     If TextBox1.TextLength < 4 Then
    3.         'Highlight the text while the error message is displayed.
    4.         TextBox1.HideSelection = False
    5.         TextBox1.SelectAll()
    6.  
    7.         MessageBox.Show("Please enter at least four characters",
    8.                         "Invalid Input",
    9.                         MessageBoxButtons.OK,
    10.                         MessageBoxIcon.Error)
    11.  
    12.         'Place the caret at the end of the text.
    13.         TextBox1.SelectionStart = TextBox1.TextLength
    14.         TextBox1.HideSelection = True
    15.  
    16.         'Don't let the user leave the control while it contains invalid input.
    17.         e.Cancel = True
    18.     End If
    19. End Sub

  3. #3

    Thread Starter
    Addicted Member Mallard8's Avatar
    Join Date
    Feb 2018
    Location
    Wales
    Posts
    171

    Re: Better coding?

    Thanks John for looking and the reply given
    If the point is the check that text entered by the user is at least four characters in length, why is it checking the contents of a Label, which doesn't accept user input, and why is it checking only for an empty String, which would not detect input of 1 to 3 characters?
    The text entered is placed in 8 labels each label shows one letter. When the player clicks the Hide button a PicturBox hides each letter in the word and any remaining labels are removed.

    Will take on board everything else.

    It's a Hangman type game for primary school age but I use carrots which are replaced by a rabbit if you guess a wrong letter instead of hanging a man.

    Name:  Labels.png
Views: 63
Size:  16.6 KB
    Learning is a never ending subject.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  



Featured


Click Here to Expand Forum to Full Width