Kelly Martens
asked on
Convert vb6 Function to C#
I converted a vb6 function to .NET. My Boss says it "isn't very .NET". First is the vb6 function, then my conversion. Can anyone suggest ways to make it "more .NET?"
Public Function CalculateCheckDigit(sInStr
' Starter requires a check digit on the end of the Serial Ship Code
' for their Optional ASN label
' Algorithm is as follows:
' Strip out the spaces and punction from the incoming string
' (e.g. take only the stuff that is numeric)
' Add all of the odd positions together (OddSum)
' Add all of the even positions together (EvenSum)
' Multiply OddSum by 3 to get Result C
' Add Result C + EvenSum, divide by 10 and get the remainder
' 10 - Remainder = Final Check digit
Dim sChar As String
Dim sStrippedString As String
Dim I As Integer
Dim iOddSum As Integer
Dim iEvenSum As Integer
Dim iResultC As Integer
Dim iRemainder As Integer
On Error GoTo Error_H
For I = 1 To Len(sInString)
sChar = Mid$(sInString, I, 1)
If IsNumeric(sChar) Then
sStrippedString = sStrippedString & sChar
End If
Next I
For I = 1 To Len(sStrippedString)
sChar = Mid$(sStrippedString, I, 1)
If I Mod 2 = 0 Then ' Even position in the string
iEvenSum = iEvenSum + Val(sChar)
Else
iOddSum = iOddSum + Val(sChar)
End If
Next I
iResultC = iOddSum * 3
iRemainder = (iResultC + iEvenSum) Mod 10
CalculateCheckDigit = IIf(iRemainder = 0, 0, 10 - iRemainder)
Exit Function
Error_H:
Handler.ErrorHandler Err, LOCATION & " - CalculateCheckDigit", True, MsgButton:=vbCritical
End Function
// my conversion
public static int CalculateCheckDigit(string
{
string Character;
string StrippedString = string.Empty;
int I = 0;
int OddSum = 0;
int EvenSum = 0;
int ResultC = 0;
int Remainder = 0;
for (I = 0; I <= InString.Length - 1; I++)
{
Character = InString.Substring(I, 1);
double OutValue = 0;
if (Double.TryParse(Character
{
StrippedString = StrippedString + Character;
}
}
for (I = 0; I <= StrippedString.Length - 1; I++)
{
Character = StrippedString.Substring(I
if ((I % 2) == 0)
{
EvenSum = EvenSum + Convert.ToInt32(Global.Val
}
else
{
OddSum = OddSum + Convert.ToInt32(Global.Val
}
}
ResultC = OddSum * 3;
Remainder = (ResultC + EvenSum) % 10;
return ((Remainder == 0) ? 0: 10 - Remainder);
}
Public Function CalculateCheckDigit(sInStr
' Starter requires a check digit on the end of the Serial Ship Code
' for their Optional ASN label
' Algorithm is as follows:
' Strip out the spaces and punction from the incoming string
' (e.g. take only the stuff that is numeric)
' Add all of the odd positions together (OddSum)
' Add all of the even positions together (EvenSum)
' Multiply OddSum by 3 to get Result C
' Add Result C + EvenSum, divide by 10 and get the remainder
' 10 - Remainder = Final Check digit
Dim sChar As String
Dim sStrippedString As String
Dim I As Integer
Dim iOddSum As Integer
Dim iEvenSum As Integer
Dim iResultC As Integer
Dim iRemainder As Integer
On Error GoTo Error_H
For I = 1 To Len(sInString)
sChar = Mid$(sInString, I, 1)
If IsNumeric(sChar) Then
sStrippedString = sStrippedString & sChar
End If
Next I
For I = 1 To Len(sStrippedString)
sChar = Mid$(sStrippedString, I, 1)
If I Mod 2 = 0 Then ' Even position in the string
iEvenSum = iEvenSum + Val(sChar)
Else
iOddSum = iOddSum + Val(sChar)
End If
Next I
iResultC = iOddSum * 3
iRemainder = (iResultC + iEvenSum) Mod 10
CalculateCheckDigit = IIf(iRemainder = 0, 0, 10 - iRemainder)
Exit Function
Error_H:
Handler.ErrorHandler Err, LOCATION & " - CalculateCheckDigit", True, MsgButton:=vbCritical
End Function
// my conversion
public static int CalculateCheckDigit(string
{
string Character;
string StrippedString = string.Empty;
int I = 0;
int OddSum = 0;
int EvenSum = 0;
int ResultC = 0;
int Remainder = 0;
for (I = 0; I <= InString.Length - 1; I++)
{
Character = InString.Substring(I, 1);
double OutValue = 0;
if (Double.TryParse(Character
{
StrippedString = StrippedString + Character;
}
}
for (I = 0; I <= StrippedString.Length - 1; I++)
{
Character = StrippedString.Substring(I
if ((I % 2) == 0)
{
EvenSum = EvenSum + Convert.ToInt32(Global.Val
}
else
{
OddSum = OddSum + Convert.ToInt32(Global.Val
}
}
ResultC = OddSum * 3;
Remainder = (ResultC + EvenSum) % 10;
return ((Remainder == 0) ? 0: 10 - Remainder);
}
Chris Stanyon
What does "not very .NET" even mean. Has he given you any other info? That's a pretty lame critique IMO.
ASKER
Chris this is my life. He is talking about regular expressions and such.
Hmm, regular expressions are also "not very .NET"..
The first thing, I can think of: Naming. Parameters and variables are lower case using the most used convention (Camel/Pascal case).
Unit tests. Which implies a class, not only a mere function. Also, more .NET like would be also implementing it on a stream.
As you work with digits, using Double.Parse seems odd.
Then there is only one loop necessary. Just skip non-numeric characters, instead of creating a new string. Then you need only on convert call.
The first thing, I can think of: Naming. Parameters and variables are lower case using the most used convention (Camel/Pascal case).
Unit tests. Which implies a class, not only a mere function. Also, more .NET like would be also implementing it on a stream.
As you work with digits, using Double.Parse seems odd.
Then there is only one loop necessary. Just skip non-numeric characters, instead of creating a new string. Then you need only on convert call.
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
I am baffled as well. I understand if he wants certain features to be used, maybe string manipulation and tertiary operators where you have used if/else but those are not .Net, but C# specific features, IMHO.
except for the onerror routine your 'VB6' code runs fine in vb.net
Hmmm, couple of things to say:
- Do not pre-declare your variables, declare them and initialise them at the same time right before using them, and give them a scope as short as possible, this for a better control on variable's lifetime.
- Use appropriate data type, values that can't be negative must be unsigned.
- Assert your parameters whenever it is possible (to detect programming errors).
- Do not pre-declare your variables, declare them and initialise them at the same time right before using them, and give them a scope as short as possible, this for a better control on variable's lifetime.
- Use appropriate data type, values that can't be negative must be unsigned.
- Assert your parameters whenever it is possible (to detect programming errors).
This is probably a bit closer to your boss's expectations:
public static int CalculateCheckDigit(string input)
{
bool even = false;
int oddSum = 0;
int evenSum = 0;
int checkDigit = 0;
for (int i = 0; i < input.Length; i++)
{
int digit;
if (int.TryParse(input.Substring(i, 1), out digit))
{
if (even)
{
evenSum += digit;
}
else
{
oddSum += digit;
}
even = !even;
}
}
checkDigit = (10 - (evenSum + (3 * oddSum)) % 10) % 10;
return checkDigit;
}
The notion of even/odd changes when you go from VB6 to C#. VB6 strings are indexed starting with 1. C# starts with 0.
No matter what your final code looks like you should add some tests that ensure that the new code is producing correct output.
No matter what your final code looks like you should add some tests that ensure that the new code is producing correct output.
I might suggest that you use the regular expression namespace to do your clean-up. You would replace all \D with an empty string. After that, your string contains only digits.
No matter what your final code looks like you should add some tests that ensure that the new code is producing correct output.
True. That's what I did. It is not air code.