Ugly solution to CC Validator Project

Hey Y’all:

I’m working on the credit card validator functions- Link Here: https://www.codecademy.com/practice/projects/credit-card-checker

For step 3, I had to create a credit card validator, and that took hours because I couldn’t get the system to validate valid3 as ‘True’. To get around that, I made a mess of loops and if else if statements. It works, but It could probably be better, and I’m not sure how to make it so. Anyone have any ideas?

The code from beyond the grave is below:

function validateCred(array){

let checkSum=0;

if (array.length % 2 === 0){for (let i= array.length-1; i>=0; i–){

if (i % 2 === 0 && 2*array[i] >= 10) {

  checkSum += (2*array[i])-9;

} else if (i % 2 === 0 && 2*array[i] < 10) {

  checkSum += 2*array[i];} 

  else if (i % 2 === 1) {

  checkSum += array[i];

  }

}

if (checkSum % 10 ===0){

return true;

} else {

return false;

}

} else {for (i= array.length-1; i>=0; i–)

  if (i % 2 === 1 && 2*array[i] >= 10) {

  checkSum += (2*array[i])-9;

} else if (i % 2 === 1 && 2*array[i] < 10) {

  checkSum += 2*array[i];} 

  else if (i % 2 === 0) {

  checkSum += array[i];

  }

}

if (checkSum % 10 ===0){

return true;

} else {

return false;

}}

I’m sure you’ve realized this but you have two for loops that loop exactly the same way and a lot of very similar code inside of those for loops. One big principle of coding is DRY(Don’t Repeat Yourself) so this is something you want to nail down early in your coding.

Now for this problem I’m guessing you chose this route because valid3 has a length that is odd. But there is a way to code a loop that works with any length, even or odd. Since you don’t know if the length will be even or odd, what if you found a way to always count from 0 and use that count to determine doubling the digit or not.

1 Like

So… A way to reverse the order inside the array. I should look that up.

That could work, just keep in mind it is not the only solution. And if you do want to modify the array, make sure you make a copy of the array and not only a copy of the reference to the array.

would ‘letNewArray= oldArray.reverse();’ Work? I’ll test it shortly

All in all I like my solution but figured it was very long, I think that this will serve its purpose well for the next steps in the exercise.

The new version works and looks much better:
Ideas still appreciated.

function validateCred(array){

let checkSum=0;

let reverseArray=array.reverse();

for ( i= 0; i < reverseArray.length; i++)

{if (i % 2 === 1 && 2*reverseArray[i] >= 10)

{checkSum += (2*reverseArray[i])-9;}

else if

(i % 2 === 1 && 2*reverseArray[i] < 10)

{checkSum += 2*reverseArray[i];}

else if (i % 2 === 0)

{checkSum += reverseArray[i];}}

if (checkSum % 10 ===0)

{return true;}

else

{return false;}}

That looks much better. You could do somethings to make it more compact but that isn’t always the goal in coding. Sometimes you want to save memory space, sometimes execution speed matters and sometimes you just want to make your code easier to read for others.

This is how I ended up writing this function

const validateCred = credNum => {
  let numSum = 0;
  for(let i = credNum.length-1; i >= 0; i--){
    let currentNum = credNum[i];
    
    //Easier to use the card length minus i in order to count the card numbers from 0 for when multiplying is needed
    if((credNum.length-1 - i) % 2 === 1){
      currentNum *= 2;
      if(currentNum > 9)
        currentNum -= 9;
    }
    numSum+= currentNum;
  }
  return numSum % 10 === 0;
}

I forgot to put this in my other reply but you have to be careful using .reverse() as it modifies the array in place. You would need to figure out a way to copy the array first but be careful there too as something like let copyArray = array doesn’t not create it’s own copy

And… It crashed because I didn’t reverse properly. Ok, I can fix that. Eventually.

Hello,

Two things,

A. Your solution is very nicely written. I need to get better at ES6 shortform notation. This is a good example for me.

B. If your interested, the first part of the function works fine, the second part isn’t working at all- it stops after index 0. If you could take a look and let me know what I left in that’s causing this I’d appreciate it.

The new code is.

let invalidCards=

function validateBatch(arrayList) {

for (i=0; i< arrayList.length; i++){

if(validateCred[i]===false){

invalidCards.push(arrayList[i])

}

}

return invalidCards;

}

Sorry. Its :
function validateBatch(arrayList) {
for (i=0; i< arrayList.length; i++){

if(validateCred(arrayList[i])===false){

invalidCards.push(arrayList[i])

}

}

return invalidCards;

}

Hmmm, it could be not using the let variable declarations in your for loops. If you don’t use let then i will be considered a global variable, which is not good since both your for loops use i.

Also I would put let invalidCards= [] inside of your validateBatch function.

Ah. That’s probably what did it. Someone else used .forEach() instead so I just did that.