0

Bouncing my head off the wall here trying to figure out a better way to handle this. I have a large input value which has three checks to check the sum of certain parts of the string in order to validate it. I'm using three try/catch blocks in one function to run the check right now and it seems to be working except for the final validation check which always seems to return true. What I'm wondering is a) is this a good method to use, b) is there a cleaner way to do this with for loop and c) why my final check is not doing anything. Any help is appreciated. I have access to jQuery and Underscore.js if that helps but I have not worked much with underscore. I made a fiddle here:

Sample Fiddle

     window.onkeyup = keyup;
    var number;

    function keyup(e) {
        number = e.target.value;
        $('#numberValue').text(number);
        // must be 10 characters long
        if (number.length !== 30) {
            return false;
        }
        number = "" + (number || "");
        // run the checksum
        var valid = false;
        try {
            var sum = (parseInt(number[0]) * 7) +
                    (parseInt(number[1]) * 3) +
                    (parseInt(number[2])) +
                    (parseInt(number[3]) * 7) +
                    (parseInt(number[4]) * 3) +
                    (parseInt(number[5])) +
                    (parseInt(number[6]) * 7) +
                    (parseInt(number[7]) * 3) +
                    (parseInt(number[8]));
            alert(((sum % 10).toFixed(0)));
            var checkDigit = ((sum % 10).toFixed(0));

            if ((number[9]) === ("" + checkDigit)) {
                alert('Our Checkdigit is valid', checkDigit);
                valid = true;
            }
        } catch (e) {
            alert('Fail for check 1!');
            valid = false;
        }
        try {
            var sum2 = (parseInt(number[13]) * 7) +
                    (parseInt(number[14]) * 3) +
                    (parseInt(number[15])) +
                    (parseInt(number[16]) * 7) +
                    (parseInt(number[17]) * 3) +
                    (parseInt(number[18]));
            alert(((sum2 % 10).toFixed(0)));
            var checkDigit2 = ((sum2 % 10).toFixed(0));

            if ((number[19]) === ("" + checkDigit2)) {
                alert('Our Checkdigit2 is valid', checkDigit2);
                valid = true;
            }
        } catch (e) {
            alert('Fail for check 2!');
            valid = false;
        }
        try {
            var sum3 = (parseInt(number[21]) * 7) +
                    (parseInt(number[22]) *3) +
                    (parseInt(number[23])) +
                    (parseInt(number[24]) * 7) +
                    (parseInt(number[25]) * 3) +
                    (parseInt(number[26]));
            alert(((sum3 % 10).toFixed(0)));
            var checkDigit3 = ((sum3 % 10).toFixed(0));

            if ((number[27]) === ("" + checkDigit3)) {
                alert('Our Checkdigit3 is valid',checkDigit3);
                valid = true;
            }
        } catch (e) {
            valid = false;
        }

        alert('All Good DUde!');
        return valid;
    }
isaac weathers
  • 1,436
  • 4
  • 27
  • 52
  • working fine for me. I might be using some wrong str, give me sample input string (input string and output expected) – joyBlanks Aug 09 '15 at 23:18
  • Made an update for a sample test number: http://jsfiddle.net/nkb5sbhv/4/ 487013675311199070109160101300 The fail is not working for the try catch if the number is bad... So in this case the 10th, 20th and 28th numbers not matching up to 3, 0 and 3 respectively with the test number should fail. This may be easier to see: 4870136753XXX9907010X1601013XX – isaac weathers Aug 09 '15 at 23:20
  • what are you expecting here ? if ((number[27], 29) === ("" + checkDigit3)) { this will always return 29 ?? – joyBlanks Aug 09 '15 at 23:41
  • I'm trying to map the position of each char/int in the input to it's relative position in the input field. – isaac weathers Aug 09 '15 at 23:43
  • But I see what you are talking about, looks like I do not need the 29 after the key pos of the input. I updated the fiddle but my catch is still not working. Wondering if I need a else to return false for each if statement within each try? | http://jsfiddle.net/nkb5sbhv/6/ – isaac weathers Aug 09 '15 at 23:50
  • 1
    I think parseInt takes a 2nd arg as radix, if you just put 29 w/o a function it will always return the 2nd arg. eg `('anything',29)` will return 29 I guess you need to check `if ((number[27], 29) === ("" + checkDigit3)){` this statement – joyBlanks Aug 09 '15 at 23:51
  • 1
    Thats kool. Now what you need is an else and also the 2nd and 3rd should have a check like if(!valid && your existing logic), else if 2nd or 3rd condition is true it will make the whole result true even if 1st was invalid – joyBlanks Aug 10 '15 at 00:00
  • The radix is optional but I did get it to fail by adding a an else statement to each try block below the if. The problem I see though is that after it does pass if I go back and start messing with it then it starts checking every key input. Probably has to do with the keyup but this is for demo. ```` if ((number[19]) === ("" + checkDigit2)) { alert('Our Checkdigit2 is valid', checkDigit2); valid = true; } else { alert('Fail for check 1!'); return false; } – isaac weathers Aug 10 '15 at 00:01
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/86564/discussion-between-joy-biswas-and-isaac-weathers). – joyBlanks Aug 10 '15 at 01:00

2 Answers2

1

From experience, you may want to separate as much math as possible in your try block. JavaScript has a weird way of handling variables and may not be doing what you think it is.

Chris
  • 377
  • 1
  • 10
  • Yes I think I should move the variable declarations out of the try logic. It seems to be working for multiple tests against the check sums. But what I am really trying to do is write all of the parseInt declarations into something a bit cleaner. Another test number that passes: 6612760563XXX3010068X1601013XX – isaac weathers Aug 09 '15 at 23:32
  • 1
    This may help alot, parseInt accepts X's: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt – Chris Aug 10 '15 at 00:01
  • Ah, so even though the radix is optional best not to leave it out. Awesome. Well I got the else statements I added to stop finally stop the try block with an extra else statement and put the radix of 10 in the parseInt declarations. Still seems like there is a better way to build this: http://jsfiddle.net/nkb5sbhv/8/ – isaac weathers Aug 10 '15 at 00:08
  • 1
    This may help, I tried to rewrite your method a little cleaner: http://jsfiddle.net/ckonowal/3qvfhnh1/ – Chris Aug 10 '15 at 00:36
1

Here is the way to do it.

I have not thrown any error, only error can be if the number is not parseable and so you can throw it if you like else if your sum checks can validate that should be good enough

window.onkeyup = keyup;
var number;

function keyup(e) {
    number = e.target.value;
    $('#numberValue').text(number);
    // must be 10 characters long
    if (number.length !== 30) {
        return false;
    }
    number = "" + (number || "");
    var valid = false;
    //try{
        var sum1 = returnSum(number,[0,1,2,3,4,5,6,7,8],[7,3,1,7,3,1,7,3,1]);
        var sum2 = returnSum(number,[13,14,15,16,17,18],[7,3,1,7,3,1]);
        var sum3 = returnSum(number,[21,22,23,24,25,26],[7,3,1,7,3,1]);
    /*
    //only if you are throwing err
    }catch(e){
        valid = false;
    }
    */

    if (number[9] === sum1 && number[19] === sum2 && number[27] === sum3) {
        console.log(sum1 +'|' + sum2 + '|' + sum3);
        valid = true;
    }


    console.log('All Good DUde!');
    return valid;
}

function myParse(n){
    return (isNaN(parseInt(n,10))) ? -1 : parseInt(n,10);
}

function returnSum(n,ind,mul){
    var acc = 0;
    var pNum = 0;
    for(var i=0; i<ind.length; i++){
        pNum = myParse(n[ind[i]]);
        if(pNum == -1){
            pNum=0;
            //throw 'error';//if you really want to throw error on not a number / or your number should fail
        }
        acc += pNum * mul[i];
    }
    return (acc%10).toFixed(0)+'';
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<h3>  Sample test number to use -- copy and paste should work </p=h3>
<p>487013675311199070109160101300</p>

<input id="searchTxt" placeholder="add numbers together">
<div id='numberValue'>Number goes here</div>

Cheers. joy

joyBlanks
  • 6,419
  • 1
  • 22
  • 47
  • I like that. Let me take a look and see if that will work. Think it has to do with the e.target and chrome spits out cannot read property of length. But is seems to work. about e.target: http://stackoverflow.com/questions/5301643/how-can-i-make-event-srcelement-work-in-firefox-and-what-does-it-mean – isaac weathers Aug 10 '15 at 00:45
  • I think that may be it. I've been staring at this too long and my head is starting to hurt. It's definitely cleaner than what I had but there's another equation and a regex check involved. Give me an hour to recheck but think you got the win. Thank you. – isaac weathers Aug 10 '15 at 01:22