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?

• Any specific case where this would be an advantage? if someone would like to automate it, it could simply put read the label. Other than that the code is pretty straight forward. i would try to reduce the duplicate code by creating a specific function for it. – Viezevingertjes Aug 4 '14 at 13:49
• With Option Strict On which you should use as default, this code does not compile. – Heslacher Aug 4 '14 at 14:24
• Why not a finger print scanner ? – bhathiya-perera Aug 4 '14 at 17:43
• @ Bhathiya-JaDogg-Perera : it is costly and need to keep database, my proposed method is simplest – Sujith Karivelil Aug 5 '14 at 3:25
• Compare the user input and expression evaluation result for confirmation. where can I find this in your code ? . – Heslacher Aug 5 '14 at 9:09

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


Enum Operand
Substract
Multiply
End Enum


and implement the Select..Case

Select Case operandType
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
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 = CalculateResult()
Return (Integer.TryParse(answer, result) AndAlso result = internalResult)
End Function

Private Function CalculateResult() As Integer
Dim result As Integer = 0

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

Return result
End Function

Public Function Generate() As String
Dim generatedCaptcha As String = String.Empty

operandType = CType(randomizer.Next(0, 3), Operand)

Select Case operandType
Case Operand.Substract
Case Operand.Multiply
End Select

firstNumber = randomizer.Next(0, 10)
secondNumber = randomizer.Next(0, 10)

Return firstNumber.ToString() & generatedCaptcha & secondNumber.ToString()
End Function
End Class


Now you need only one label to show the captcha which I have named captchaLabel and your code looks fine and tidy

Dim _captcha As New Captcha()

Private Sub Generate_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Generate.Click

End Sub

Private Sub check_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles check.Click

MessageBox.Show("Successfully verified")
Else
MessageBox.Show("Verification Failed")
End If

End Sub


Side Note As I see that you are comming from VB6, I want to encourage you to learn the .NET way of these things. So skip using MsgBox(),CInt(), Mid() etc, as these are only kept to stay backward compatible.

• This is a really good review. I thought about mentioning a captcha class, but figured I was already throwing a lot at OP. In hindsight, I should have. – RubberDuck Aug 5 '14 at 15:48
• You should use Option Strict and Option Explicit. They magically turns runtime errors into compiler errors.
• I'm not a fan of seeing a bunch of logic that isn't related to the GUI in GUI event procedures. Separate the logic into separate subroutines and call them from the click events. Eventually you'll want to call this logic from somewhere else. Let's remove the temptation to raise events instead of calling procedures now. The first event procedure should look like this:

Private Sub Generate_Click()
End Sub

• Variables should be camelCased. Dim Rand should be Dim rand.

• If you need a comment to explain what a variable is, you should rename that variable.

If op = 0 Then ' Operator selection

• Same line, what you want here is a Select Case. More about the Select statement.

• Define an enumeration for your operationSelector values. It will clean up your Selects.

 Private Enum Operation
subtract
multiply
End Enum

• In general, this should be split up into smaller functions/subs. Each one should have a single responsibility.

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
Dim operationSelector As Integer

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

SetLblOp operationSelector

End Sub

Private Sub SetLblOp(operationSelector as Operation)

Select Case operationSelector
lblop.Text = "+"
Case Operation.subtract
lblop.Text = "-"
Case Operation.multiply
lblop.Text = "*"
End Select
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

MsgBox("Successfully varified")
Else
MsgBox("Varification Faild")
End If
End Sub

Private Function CalculateCaptcha(operationSelector As Operation) As Integer
Select Case operationSelector