HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Numeric selfmade CAPTCHA in Windows application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
captchaapplicationnumericselfmadewindows

Problem

Nowadays many sites introduce CAPTCHAs (Completely Automated Public Turing test to tell Computers and Humans Apart) for securing their sites. I wish to introduce the technique in windows forms, as expression evaluation CAPTCHA.

Approach:

  • Two randomly generated integers are loaded into two different labels



  • Place a random operator (+,-,*) in between them



  • Users are requested to provide the calculated result in a text box



  • Compare the user input and expression evaluation result for confirmation.



Private Sub Generate_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Generate.Click
    ' This function will generate the Numeric Captcha 
    ' with two random number and one operator
    Dim Rand As New System.Random
    op = Rand.Next(0, 3)
    lblfirst.Text = Rand.Next(0, 10) 'first random number
    lblsecond.Text = Rand.Next(0, 10) 'second random number
    If op = 0 Then ' Operator selection
        lblop.Text = "+"
    ElseIf op = 1 Then
        lblop.Text = "-"
    ElseIf op = 2 Then
        lblop.Text = "*"
    End If
End Sub

Private Sub check_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles check.Click
    'Validating the user input with Random CAPTCHA
    Dim output As Integer
    If op = 0 Then ' choosing operator based on op
        output = CInt(lblfirst.Text) + CInt(lblsecond.Text)
    ElseIf op = 1 Then
        output = CInt(lblfirst.Text) - CInt(lblsecond.Text)
    ElseIf op = 2 Then
        output = CInt(lblfirst.Text) * CInt(lblsecond.Text)
    End If
    If output = CInt(txtResult.Text) Then
        MsgBox("Successfully varified")
    Else
        MsgBox("Varification Faild")
    End If
End Sub


Are there any suggestions on improving the technique?

Solution

Option Explicit On

From what's an option strict and explicit?

Option Strict "restricts implicit data type conversions to only widening conversions". See here. With this option enabled, you can't accidentally convert one data type to another that is less precise (e.g. from an Integer to a Byte). Again, an option that should be turned on by default.

So after turning Option Strict On you will get 2 errors at these lines

lblfirst.Text = Rand.Next(0, 10) 'first random number
lblsecond.Text = Rand.Next(0, 10) 'second random number


stating "Option Strict On prohibits implicit conversion of Integer to String" (my translation from german).

Now you might think that's a bad thing, but it isn't. It will just show you where you need some work to do.

And for this 2 errors it is also quite easy, you need only to call the ToString() method of the Integer which is returned by calling the Rand.Next() method and there is the next problem.

Variables should use camelCase for their names and also the names should be descriptive so you won't need any comments for these variables anymore, so let us do it right and rename op to operandType and also Rand to randomizer

Dim randomizer As New System.Random
Dim operandType as Integer = 0

operandType = randomizer.Next(0, 3)
lblfirst.Text = randomizer.Next(0, 10).ToString() 'first random number
lblsecond.Text = randomizer.Next(0, 10).ToString() 'second random number


But wait, you are creating Integers convert them to Strings which you assign to the Text property of the labels which you are converting to Integers again inside the check_Click() method. That can be done better. Let declare some variables on class level to do this.

Private firstNumber As Integer = 0
Private secondNumber As Integer = 0


and change the lines above to

firstNumber = randomizer.Next(0, 10)
lblfirst.Text = firstNumber.ToString() 'first random number

secondNumber = randomizer.Next(0, 10)
lblsecond.Text = secondNumber.ToString() 'second random number


now we can use firstNumber and secondNumber in the check_Click method without using the String to Integer conversation (I will later talk about this also).

But wait, we can do better... Why don't we calculate the desired result at the time we generate the captcha ? Let us declare another Integer variable to hold the result

Private internalResult As Integer = 0


and use it

If operandType = 0 Then ' Operator selection
    internalResult = firstNumber + secondNumber
ElseIf operandType = 1 Then
    internalResult = firstNumber - secondNumber
ElseIf operandType = 2 Then
    internalResult = firstNumber * secondNumber
End If


So looking at this If..ElseIf..ElseIf I see 2 things, operandType is in the Range[0..2] which should be expressed as an enum (which I will lend from ckuhn203's answer) and the If..ElseIf.. should be a Select Case . We need also to change

Private operandType As Integer


to

Private operandType As Operand


next we add the enum

Enum Operand
    Add
    Substract
    Multiply
End Enum


and implement the Select..Case

Select Case operandType
    Case Operand.Add
        internalResult = firstNumber + secondNumber
    Case Operand.Substract
        internalResult = firstNumber - secondNumber
    Case Operand.Multiply
        internalResult = firstNumber * secondNumber
End Select


Now let us talk about conversion from String to Integer by taking a look at the last few lines of your check_Click() method

If output = CInt(txtResult.Text) Then
    MsgBox("Successfully varified")
Else
    MsgBox("Varification Faild")
End If


You are calling CInt(txtResult.Text). Assume the user will enter "lala" in the textbox. This will result in an InvalidCastException. A better way would be to use Integer.TryParse() which as the name implies tries to parse a String to an Integer. If this succeeds the method returns true otherwise it will return false. So let us use it

If (Integer.TryParse(txtResult.Text, output) AndAlso output = internalResult) Then
    MsgBox("Successfully verified")
Else
    MsgBox("Verification failed")
End If


Wait, what is this AndAlso ? AndAlso performs a short-circuiting logical conjunction on two expressions. So this means, that the right part of the AndAlso will only be evaluated, if the left part is true.

As we have, putting aside the last part, only code which belongs to the captcha generating and checking, so it would be a good thing to create a class of it

```
Public Class Captcha

Private Enum Operand
Add
Substract
Multiply
End Enum

Private firstNumber As Integer = 0
Private secondNumber As Integer = 0
Private internalResult As Integer = 0
Private operandType As Operand
Private result As Integer = 0
Private randomizer As New Random()

Public Function IsValid(answer As String) As Boolean
internalResult = CalculateR

Code Snippets

lblfirst.Text = Rand.Next(0, 10) 'first random number
lblsecond.Text = Rand.Next(0, 10) 'second random number
Dim randomizer As New System.Random
Dim operandType as Integer = 0

operandType = randomizer.Next(0, 3)
lblfirst.Text = randomizer.Next(0, 10).ToString() 'first random number
lblsecond.Text = randomizer.Next(0, 10).ToString() 'second random number
Private firstNumber As Integer = 0
Private secondNumber As Integer = 0
firstNumber = randomizer.Next(0, 10)
lblfirst.Text = firstNumber.ToString() 'first random number

secondNumber = randomizer.Next(0, 10)
lblsecond.Text = secondNumber.ToString() 'second random number
Private internalResult As Integer = 0

Context

StackExchange Code Review Q#59009, answer score: 5

Revisions (0)

No revisions yet.