8
\$\begingroup\$

I've found an explanation on Wikipedia on how to validate an IBAN. Since the number generated from the transformation can cause overflow with all data types available in VBA, I've worked with String.

I'd like to have someone else review. Thanks in advance!

From Wikipedia:

  1. Check that the total IBAN length is correct as per the country. If not, the IBAN is invalid .
  2. Move the four initial characters to the end of the string .
  3. Replace each letter in the string with two digits, thereby expanding the string, where A = 10, B = 11, ..., Z = 35.
  4. Interpret the string as a decimal integer and compute the remainder of that number modulo 97.
Public Function isValidIBAN(IBAN As String) As Boolean

    isValidIBAN = False

    Dim Country         As String
    Dim CountryLenght   As Integer
    Dim tempStr         As String
    Dim c               As String
    Dim o               As Long
    Dim newStr          As Variant
    Const Modder        As Integer = 97

    If IBAN = vbNullString Then Exit Function
    Country = Left(IBAN, 2)
    CountryLenght = 0
    On Error Resume Next
    CountryLenght = Application.WorksheetFunction.VLookup(Country, Foglio3.Range("A:D"), 4, 0) 'This Search in a table --- Country|someVal|someVal|IBAN lenght
    On Error GoTo 0
    If Len(IBAN) <> CountryLenght Then Exit Function

    'move first 4 chars to right
    tempStr = Right(IBAN, Len(IBAN) - 4) & Left(IBAN, 4)
    'loop throught single char in tempStr and if not numeric return 10 based number from letter
    'use string in place of number to store new-generated "IBAN"
    For o = 1 To Len(tempStr)
        c = Mid(tempStr, o, 1)
        If Not IsNumeric(c) Then
            newStr = newStr & CStr(Range(c & 1).Column + 9)
        Else
            newStr = newStr & CStr(c)
        End If
    Next o

    c = vbNullString
    ' perform primary school' style division - digit by digit
    For o = 1 To Len(newStr)
        c = c & Mid(newStr, o, 1)
        myStr = myStr & CStr(Int(CLng(c) / Modder))
        'if is the last char in str check if mod is 1 - Only fired once
        If o = Len(str) Then
            isValidIBAN = ((CLng(c) Mod Modder) = 1)
            Exit Function
        End If
        c = IIf(CLng(c) < Modder, c, CLng(c) Mod Modder)
    Next o
    End Function
\$\endgroup\$
1
  • \$\begingroup\$ Micro-review - misspelt CountryLength. \$\endgroup\$ Commented Feb 18, 2020 at 13:12

1 Answer 1

5
\$\begingroup\$

On first sight, your code looks reasonably short. Nevertheless, no matter how short the code is, there's always something to improve.

The function name and signature are perfect. I though about passing the string as ByVal, but that might be less efficient since the string might have to be copied then.

Initializing the return value of the function to False at the very beginning is good style. For a function that validates something, any accidental return should say "not valid". This is a commonly found pattern, especially in security-related code.

Some of the variable names start with an uppercase letter and some with a lowercase letter. I don't see any reason to mix these two styles. You should either start all variable names with an uppercase letter, or all with a lowercase letter.

Typo: Lenght should be Length instead. It seems the Microsoft IDE lacks a spell checker. That should really be fixed on Microsoft's side.

The variable name tempStr does not say much about the purpose of that variable. A better name would be ReorderedIBAN.

The variable name o is strange as well. For cases like here, the variable name i is more common. This name is an abbreviation for index.

The variable name newStr is confusing because its type is not String but Variant.

After each line containing Exit Function, I would put an empty line, to start a new paragraph. This groups the code lines more logically.

It's unfortunate that this simple function needs a whole spreadsheet just to look up the IBAN length by country name. I'd rather encode all the length requirements in a single string, like this:

' See https://en.wikipedia.org/wiki/International_Bank_Account_Number#IBAN_formats_by_country
LengthByCountry = "AL28 AN24 AT20 AZ28 BH22 ..."

Given this string, you could search like this:

LengthIndex = Index(LengthByCountry, Left(IBAN, 2))
If LengthIndex Mod 5 <> 1 Then Exit Function

CountryLength = Mid(LengthByCountry, LengthIndex + 2, 2)
If Len(IBAN) <> CountryLength Then Exit Function

And if you made it this far, you could also validate that the IBAN matches the BBAN Format that is given in the Wikipedia article.

When you loop over the characters of the ReorderedIBAN, you use a spreadsheet lookup again. I'm sure VBA has a built-in function for getting the character code from the first character of a string, but I couldn't find it. Usually it is called ASC, ORD, CODE.

Instead of creating new strings all the time, you could also do the Mod 97 math directly on the digits as you convert them. The idea is:

Result = 0
For i = 1 To Len(ReorderedIBAN)
    ch = Mid(ReorderedIBAN, i, 1)

    If IsNumeric(ch) Then
        n = Asc(ch) - Asc("0")
    Else
        n = Asc(ch) - Asc("A") + 10
        If n < 1 Or n > 26 Then Exit Function
    End If

    If Result < 10 Then
        Result = (10 * Result + n) Mod 97
    Else
        Result = (100 * Result + n) Mod 97
    End If
Next i

The above code takes care not to use strings but rather only simple arithmetic operations, since the latter are usually faster than string operations.

Your code currently may or may not handle lowercase letters correctly. That depends on the part of the Excel spreadsheet that you omitted from your question. If in doubt, just call UCase on the ReorderedIBAN before looping over it.

To ensure your code works as intended, you should place a Sub TestIsValidIban next to it that contains example IBANs and whether they validate or not. If any of them does not validate, an error should occur. Or does VBA have a unit test framework? Then, use that instead. Have a look at this example code at Rosetta Code, which contains a half-automatic test; you still have to inspect its output manually.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Would using the lookup string not hurt maintainability? I expect that the values will change - if only because new countries come into existence. \$\endgroup\$
    – guntbert
    Commented Feb 18, 2020 at 19:04
  • 2
    \$\begingroup\$ I weighed the maintainability cost against having to copy the two spreadsheet areas into every VBA application where you want to use the IBAN validator. Having all the code in a single place sounded more maintainable and error-proof than having to update the validation code from time to time. \$\endgroup\$ Commented Feb 18, 2020 at 19:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.