2
\$\begingroup\$

For a project, I need to validate a VAT number against the correct algorithm. I found a library for this here (freeware, so I should be able to reuse it) that I'm grateful exists. However, I feel like it could be improved upon in a few ways, but I'm not sure which.

The piece of code is:

// Check the string against the regular expressions for all types of VAT numbers
for (i = 0; i < vatexp.length; i++) {

    // Have we recognised the VAT number?
    if (vatexp[i].test(vatNumber)) {

        // Yes - we have
        var cCode = RegExp.$1;                             // Isolate country code
        var cNumber = RegExp.$2;                           // Isolate the number
        if (cCode.length == 0) cCode = defCCode;           // Set up default country code

        // Now perform the check digit calculation according to the national rules.
        //switch (cCode) {
        //    case "AT":
        //        valid = ATVATCheckDigit(cNumber);
        //        break;
        //    case "BE":
        //        valid = BEVATCheckDigit(cNumber);
        //        break;
        //    case "BG":
        //        valid = BGVATCheckDigit(cNumber);
        //        break;
        //    case "CHE":
        //        valid = CHEVATCheckDigit(cNumber);
        //        break;
        //    case "CY":
        //        valid = CYVATCheckDigit(cNumber);
        //        break;
        //    case "CZ":
        //        valid = CZVATCheckDigit(cNumber);
        //        break;
        //    case "DE":
        //        valid = DEVATCheckDigit(cNumber);
        //        break;
        //    case "DK":
        //        valid = DKVATCheckDigit(cNumber);
        //        break;
        //    case "EE":
        //        valid = EEVATCheckDigit(cNumber);
        //        break;
        //    case "EL":
        //        valid = ELVATCheckDigit(cNumber);
        //        break;
        //    case "ES":
        //        valid = ESVATCheckDigit(cNumber);
        //        break;
        //    case "EU":
        //        valid = EUVATCheckDigit(cNumber);
        //        break;
        //    case "FI":
        //        valid = FIVATCheckDigit(cNumber);
        //        break;
        //    case "FR":
        //        valid = FRVATCheckDigit(cNumber);
        //        break;
        //    case "GB":
        //        valid = UKVATCheckDigit(cNumber);
        //        break;
        //    case "GR":
        //        valid = ELVATCheckDigit(cNumber);
        //        break;
        //    case "HR":
        //        valid = HRVATCheckDigit(cNumber);
        //        break;
        //    case "HU":
        //        valid = HUVATCheckDigit(cNumber);
        //        break;
        //    case "IE":
        //        valid = IEVATCheckDigit(cNumber);
        //        break;
        //    case "IT":
        //        valid = ITVATCheckDigit(cNumber);
        //        break;
        //    case "LT":
        //        valid = LTVATCheckDigit(cNumber);
        //        break;
        //    case "LU":
        //        valid = LUVATCheckDigit(cNumber);
        //        break;
        //    case "LV":
        //        valid = LVVATCheckDigit(cNumber);
        //        break;
        //    case "MT":
        //        valid = MTVATCheckDigit(cNumber);
        //        break;
        //    case "NL":
        //        valid = NLVATCheckDigit(cNumber);
        //        break;
        //    case "NO":
        //        valid = NOVATCheckDigit(cNumber);
        //        break;
        //    case "PL":
        //        valid = PLVATCheckDigit(cNumber);
        //        break;
        //    case "PT":
        //        valid = PTVATCheckDigit(cNumber);
        //        break;
        //    case "RO":
        //        valid = ROVATCheckDigit(cNumber);
        //        break;
        //    case "RS":
        //        valid = RSVATCheckDigit(cNumber);
        //        break;
        //    case "RU":
        //        valid = RUVATCheckDigit(cNumber);
        //        break;
        //    case "SE":
        //        valid = SEVATCheckDigit(cNumber);
        //        break;
        //    case "SI":
        //        valid = SIVATCheckDigit(cNumber);
        //        break;
        //    case "SK":
        //        valid = SKVATCheckDigit(cNumber);
        //        break;
        //}
        valid = window[cCode + "VATCheckDigit"](cNumber);
        // Load new VAT number back into the form element
        if (valid) {
             valid = vatNumber;
        }

        // We have found that the number is valid - break from loop
        break;
    }
}

As you can see, I already replaced that ungodly switch statement with a single function call, but I'm wondering if there's a way to rewrite the for...if...break statement so it's no longer a for loop, but while it's still human-readable.

vatexp is a 44 regex array that has a bunch of regexes in there to determine what country the number is for. I'm hoping there's a way to write this code so it does not have to do a long loop.

The full function:

function checkVATNumber(toCheck) {

// Array holds the regular expressions for the valid VAT number
var vatexp = new Array();

// To change the default country (e.g. from the UK to Germany - DE):
//    1.  Change the country code in the defCCode variable below to "DE".
//    2.  Remove the question mark from the regular expressions associated with the UK VAT number: 
//        i.e. "(GB)?" -> "(GB)"
//    3.  Add a question mark into the regular expression associated with Germany's number 
//        following the country code: i.e. "(DE)" -> "(DE)?"

var defCCode = "BE";

// Note - VAT codes without the "**" in the comment do not have check digit checking.

vatexp.push(/^(AT)U(\d{8})$/);                           //** Austria
vatexp.push(/^(BE)?(0?\d{9})$/);                         //** Belgium 
vatexp.push(/^(BG)(\d{9,10})$/);                         //** Bulgaria 
vatexp.push(/^(CHE)(\d{9})(MWST)?$/);                    //** Switzerland
vatexp.push(/^(CY)([0-5|9]\d{7}[A-Z])$/);                //** Cyprus
vatexp.push(/^(CZ)(\d{8,10})(\d{3})?$/);                 //** Czech Republic
vatexp.push(/^(DE)([1-9]\d{8})$/);                       //** Germany 
vatexp.push(/^(DK)(\d{8})$/);                            //** Denmark 
vatexp.push(/^(EE)(10\d{7})$/);                          //** Estonia 
vatexp.push(/^(EL)(\d{9})$/);                            //** Greece 
vatexp.push(/^(ES)([A-Z]\d{8})$/);                       //** Spain (National juridical entities)
vatexp.push(/^(ES)([A-H|N-S|W]\d{7}[A-J])$/);            //** Spain (Other juridical entities)
vatexp.push(/^(ES)([0-9|Y|Z]\d{7}[A-Z])$/);              //** Spain (Personal entities type 1)
vatexp.push(/^(ES)([K|L|M|X]\d{7}[A-Z])$/);              //** Spain (Personal entities type 2)
vatexp.push(/^(EU)(\d{9})$/);                            //** EU-type 
vatexp.push(/^(FI)(\d{8})$/);                            //** Finland 
vatexp.push(/^(FR)(\d{11})$/);                           //** France (1)
vatexp.push(/^(FR)([(A-H)|(J-N)|(P-Z)]\d{10})$/);        // France (2)
vatexp.push(/^(FR)(\d[(A-H)|(J-N)|(P-Z)]\d{9})$/);       // France (3)
vatexp.push(/^(FR)([(A-H)|(J-N)|(P-Z)]{2}\d{9})$/);      // France (4)
vatexp.push(/^(GB)(\d{9})$/);                            //** UK (Standard)
vatexp.push(/^(GB)(\d{12})$/);                           //** UK (Branches)
vatexp.push(/^(GB)(GD\d{3})$/);                          //** UK (Government)
vatexp.push(/^(GB)(HA\d{3})$/);                          //** UK (Health authority)
vatexp.push(/^(GR)(\d{8,9})$/);                          //** Greece 
vatexp.push(/^(HR)(\d{11})$/);                           //** Croatia 
vatexp.push(/^(HU)(\d{8})$/);                            //** Hungary 
vatexp.push(/^(IE)(\d{7}[A-W])$/);                       //** Ireland (1)
vatexp.push(/^(IE)([7-9][A-Z\*\+)]\d{5}[A-W])$/);        //** Ireland (2)
vatexp.push(/^(IE)(\d{7}[A-W][AH])$/);                   //** Ireland (3)
vatexp.push(/^(IT)(\d{11})$/);                           //** Italy 
vatexp.push(/^(LV)(\d{11})$/);                           //** Latvia 
vatexp.push(/^(LT)(\d{9}|\d{12})$/);                     //** Lithunia
vatexp.push(/^(LU)(\d{8})$/);                            //** Luxembourg 
vatexp.push(/^(MT)([1-9]\d{7})$/);                       //** Malta
vatexp.push(/^(NL)(\d{9})B\d{2}$/);                      //** Netherlands
vatexp.push(/^(NO)(\d{9})$/);                            //** Norway (not EU)
vatexp.push(/^(PL)(\d{10})$/);                           //** Poland
vatexp.push(/^(PT)(\d{9})$/);                            //** Portugal
vatexp.push(/^(RO)([1-9]\d{1,9})$/);                     //** Romania
vatexp.push(/^(RU)(\d{10}|\d{12})$/);                    //** Russia
vatexp.push(/^(RS)(\d{9})$/);                            //** Serbia
vatexp.push(/^(SI)([1-9]\d{7})$/);                       //** Slovenia
vatexp.push(/^(SK)([1-9]\d[(2-4)|(6-9)]\d{7})$/);        //** Slovakia Republic
vatexp.push(/^(SE)(\d{10}01)$/);                         //** Sweden

// Load up the string to check
var vatNumber = toCheck.toUpperCase();

// Remove spaces etc. from the VAT number to help validation
var chars = [" ", "-", ",", "."];
for (var i = 0; i < chars.length; i++) {
    while (vatNumber.indexOf(chars[i]) != -1) {
        vatNumber = vatNumber.slice(0, vatNumber.indexOf(chars[i])) + vatNumber.slice(vatNumber.indexOf(chars[i]) + 1);
    }
}

// Assume we're not going to find a valid VAT number
var valid = false;

// Check the string against the regular expressions for all types of VAT numbers
for (i = 0; i < vatexp.length; i++) {

    // Have we recognised the VAT number?
    if (vatexp[i].test(vatNumber)) {

        // Yes - we have
        var cCode = RegExp.$1;                             // Isolate country code
        var cNumber = RegExp.$2;                           // Isolate the number
        if (cCode.length == 0) {
            cCode = defCCode;
        }           // Set up default country code

        // Now perform the check digit calculation according to the national rules.
        //Each function starts with 2-3 char code that indicates the country the function is for.
        //this function replaces the original 100 line switch with 37 cases that the original author used.
        valid = window[cCode + "VATCheckDigit"](cNumber);
        // Load new VAT number back into the form element
        if (valid) {
            var splitVat = new Array();
            splitVat[0] = cCode;
            splitVat[1] = cNumber;
            valid = splitVat;
        }

        // We have found that the number is valid - break from loop
        break;
    }
}

// Return with either an error or the reformatted VAT number
return valid;
}

Does anyone have a suggestion on if and how I can replace this for loop?

NOTE: this particular code is not the most recent one. I just fixed 2 bugs related to my simplification (2 methods that didn't follow the naming convention and had to be renamed or redirected). The question still remains the same.

\$\endgroup\$
8
  • \$\begingroup\$ You didn't give us a bunch of variables... First whats vatNumber, defCode, etc \$\endgroup\$
    – megawac
    Commented Jul 23, 2014 at 14:22
  • \$\begingroup\$ vatNumber is the argument of the function. defCode is the default country identification code for numbers that don't have a code specified (i.e. those from Belgium). \$\endgroup\$
    – Nzall
    Commented Jul 23, 2014 at 14:54
  • \$\begingroup\$ Then you should include the entire functions mate, not just a snippet. And also give a way to call the function \$\endgroup\$
    – megawac
    Commented Jul 23, 2014 at 15:00
  • \$\begingroup\$ I'm working on an answer. Is it critical that Austria match to AT despite being ATU, or is it okay to just change the function call to ATUVAT... (or alias)? \$\endgroup\$
    – Schism
    Commented Jul 23, 2014 at 15:01
  • \$\begingroup\$ I'll add the full function in addition to the snippet. \$\endgroup\$
    – Nzall
    Commented Jul 23, 2014 at 15:01

2 Answers 2

3
\$\begingroup\$

First of all, I don't think you should worry too much about the loop in particular. It may seem a little bit naïve, but it's easy to understand and easy to add new patterns if need be.

Your refactoring cleans it up a lot (like, really a lot! Kudos), but the suspect part (IMO) are the many, many functions just floating around in the global namespace. I imagine quite a few of them actually share the same check digit calculation (or at least the same operations).

Anyway, it's basically a question of data structure and reuse, I'd say. For instance, you could maybe use a 2-char country code as a key. The only trick is, as far as I can see, the Belgian VAT numbers that don't necessarily start with "BE". But setting that aside for a moment, you could do something like:

var countryCode = value.toUpperCase().slice(0, 2),
    regexps = vatPatterns[countryCode];

if( !regexps ) {
  // if there are no matching patterns for the countryCode
  // the VAT number is invalid (or it's Belgian)...

} else {
  // loop through the country-specific regex patterns. Many only
  // have a single pattern, but for instance Spain has several
  // so there's going to be a loop and an array somewhere.

}

But defining vatPatterns as an object containing arrays of regexes might get a little dull, with a lot of repetition. Besides, you still need the check digit validators.

It'd might be worth it to encapsulate pattern and validator function in a constructor/class, like for instance:

function VATNumber(description, pattern, validator) { ... }

which would make it simpler to maintain the list (hopefully).

But is it worth it?

All that said, it's hard to know what's really worth spending time on here. If it works, it works, and perhaps that's good enough. I'd still make sure that the code isn't just injected straight into the global scope, but I might leave it alone after that (or at least try very hard to).

These sorts of things don't necessarily lend themselves to a clean solution, since every country has its own practices for, well, everything. So, sure, every type of VAT number is no doubt very logical on its own, but if it has little in common with the others, the code will become a branching tangle no matter what you do (or it'll become so perfectly abstract that maintaining it is complex). File this under "ain't pretty but it works" and call it a day - provided it really does work, of course.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Country codes can be three characters (CHE, ATU). \$\endgroup\$
    – Schism
    Commented Jul 23, 2014 at 14:54
  • 1
    \$\begingroup\$ @Schism True, but you'll note that for the list of countries given above, 2 letters are sufficient to remove any ambiguity; AT and CH is enough. Is it perfect? No, of course not, but it works. Again, the point in my answer is that a task like this - checking many countries differing VAT codes - doesn't lend itself a single, perfect, one-size-fits-all implementation. It'll get messy regardless. To my eyes, trying to account for 2 and 3 letter codes just adds more complexity, even though it works fine with just 2-letter codes \$\endgroup\$
    – Flambino
    Commented Jul 23, 2014 at 15:37
  • \$\begingroup\$ @Schism according to ISO_3166-1 country codes are either 2 or 3 letters, every country has a 2 and a 3 letter identifier. So you either always use 2 letters or always use 3 letters. \$\endgroup\$
    – Pieter B
    Commented Jul 29, 2014 at 8:50
3
\$\begingroup\$

First of all, please strongly consider @Flambino's advice; it's excellent.

That said, I don't imagine valid VAT patterns change often. For this reason, I propose condensing each country's pattern into a single regular expression.

Since you're so intent on eliminating the loop: notice that if my VAT number is "GBGD123", then based on the prefix, it can't possibly be a (e.g.) French VAT code.

I would try matching the country code first, and then match the rest based on that.

Consider something like this. You'll notice though that because the various types are condensed, you lose e.g. the comments on UK VAT codes. You'll need to decide if this is worthwhile to you.

function checkVATNumber(toCheck) {
    var vatNumber = toCheck.toUpperCase();
    var defCCode = "BE";
    var countryRE = {
        ATU: /^\d{8}$/,
        BE : /^0?\d{9}$/,
        BG : /^\d{9,10}$/,
        CHE: /^\d{9}(?=MWST$|$)/,
        CY : /^[0-59]\d{7}[A-Z]$/,
        CZ : /^\d{8,10}(?=\d{3}$|$)/,
        DE : /^[1-9]\d{8}$/,
        DK : /^\d{8}$/,
        EE : /^10\d{7}$/,
        EL : /^\d{9}$/,
        ES : /^(?:[A-Z]\d{8}|[A-HN-SW]\d{7}[A-J]|[0-9YZKLMX]\d{7}[A-Z])$/,
        EU : /^\d{9}$/,
        FI : /^\d{8}$/,
        FR : /^(?:\d|[A-HJ-NP-Z]){2}\d{9}$/,
        GB : /^(?:\d{9}(?:\d{3})?|(?:GD[0-4]|HA[5-9])\d{2})$/,
        GR : /^\d{8,9}$/,
        HR : /^\d{11}$/,
        HU : /^\d{8}$/,
        IE : /^(?:\d{7}[A-W][AH]?|[789][A-Z*+]\d{5}[A-W])$/,
        IT : /^\d{11}$/,
        LV : /^\d{11}$/,
        LT : /^\d{9}(?:\d{3})?$/,
        LU : /^\d{8}$/,
        MT : /^[1-9]\d{7}$/,
        NL : /^\d{9}B\d{2}$/,
        NO : /^\d{9}$/,
        PL : /^\d{10}$/,
        PT : /^\d{9}$/,
        RO : /^[1-9]\d{1,9}$/,
        RU : /^\d{10}(?:\d{2}?)$/,
        RS : /^\d{9}$/,
        SI : /^[1-9]\d{7}$/,
        SK : /^[1-9]\d[2346-9]\d{7}$/,
        SE : /^\d{10}01$/
    };
    var cCode, cNumber;

    // expensive! consider the alternatives: /^(?:ATU|BE|...)$/
    // or, just match /^(?:ATU|CHE|[A-Z]{2})$/. If you do this, you'll need to check
    // that countryRE[cCode] exists before exec'ing later.
    if (RegExp("^(" + Object.keys(countryRE).join("|") + ")(.+)$").test(vatNumber)) {
        cCode = RegExp.$1;
        cNumber = RegExp.$2;
    } else {
        // couldn't match a country; fallback to default
        cCode = defCCode;
        cNumber = vatNumber;
    }
    cNumber = countryRE[cCode].exec(cNumber);

    // if we match the pattern for this country, AND the check function is good 
    // (e.g. checksum is valid), then return the reformatted VAT number.
    if (cNumber && window[cCode + "VATCheckDigit"](cNumber[0]))
        return [cCode, cNumber[0]];

    // either the pattern is invalid for the country, or the check function failed.
    return false;
}

In summary, this code will:

  1. Try to match the country code. If none matches, falls back to default.
  2. Lookup the appropriate RegExp for that country code and attempt a match.
  3. If the match succeeds, attempt to verify the number with the appropriate VATCheckDigit function.

There aren't any loops, but the country test is expensive; I listed alternatives in a comment.

Of note for the RegExp's: - In the original, Switzerland has three groupings, but only the second is used as the cNumber. To emulate this without grouping, I use a positive lookahead ((?=...)). - I changed GB GD & GB HA according to their spec; since GD must be followed by 0...499 and HA by 500...999, I incorporated that into the match.

\$\endgroup\$

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.