HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Credit card validation with reversed array

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withcreditarrayreversedvalidationcard

Problem

I want a review of this code.

 Credit card Before :- ";
  for($i=0;$i";
  echo " Credit card after applaying mod 10 Algorthim :- ";
  //start converting the number
  for($i=0;$i 9 ){

          $array[$i] -= 9;
        }
      }
      //suming all the resulted number
      $total +=$array[$i];
      if($i%2==1)
        echo "".$array[$i]."";
      else
        echo $array[$i];
    }
    echo "";
      if($total % 10 ==0){
        echo "This creidet card is valid";
      }else{
        echo "This creidet card is not valid";
      }
?>

Solution

There's a few glaring issues with your code. Namely your lack of consistent styling. Secondly, you're not doing certain things correctly (iterating over an array rather than imploding it). As this is a code review, I will not be touching on the correctness of the algorithm. Below you'll find two pieces of code - one which I've commented and corrected, and a second which I've removed my comments and old code.

With comments:

 Credit card Before :- ";

# No need to do a for loop, just use implode to make the array into a string
/*for ( $i = 0; $i ";*/
echo " Credit card after applaying mod 10 Algorthim :- ";

# Start converting the number
# You're doing a sizeof() on every single loop. Just cache it in a $size variable
$size = count($numbers_array);
for ( $i = 0; $i  9 ) {
            $numbers_array[$i] -= 9;
        }

        echo "" . $numbers_array[$i] . "";
    } else {
        echo $numbers_array[$i]
    }

    $total += $numbers_array[$i];

    # You're doing the same calculation twice (as above). Why not just echo it up there?
    /*if ( $i % 2 == 1 ){
        echo "" . $numbers_array[$i] . "";
    } else {
        echo $numbers_array[$i];
    }*/
}

echo "";
if ( $total % 10 == 0 ) {
    echo "This credit card is valid";
} else {
    echo "This credit card is not valid";
}


With no comments:

 Credit card Before :- " . implode("", $numbers_array) . 
     " Credit card after applaying mod 10 Algorthim :- ";

# Start converting the number
$size = count($numbers_array);
for ( $i = 0; $i  9 ) {
            $numbers_array[$i] -= 9;
        }

        echo "" . $numbers_array[$i] . "";
    } else {
        echo $numbers_array[$i]
    }

    $total += $numbers_array[$i];
}

if ( $total % 10 == 0 ) {
    echo "This credit card is valid";
} else {
    echo "This credit card is not valid";
}

Code Snippets

<?php

# Grab the numbers array and reverse it. 
# Don't just name your variable $array. Name it something meaningful.
$numbers_array = array_reverse( $_POST['array'] );

echo "<br> Credit card Before :- ";

# No need to do a for loop, just use implode to make the array into a string
/*for ( $i = 0; $i < sizeof( $array ); $i++ ){
    echo $array[$i];
}*/
echo implode("", $numbers_array);

# Just do one echo instead of two
/*echo "<br>";*/
echo "<br><br> Credit card after applaying mod 10 Algorthim :- ";

# Start converting the number
# You're doing a sizeof() on every single loop. Just cache it in a $size variable
$size = count($numbers_array);
for ( $i = 0; $i < $size; $i++ ) {
    if ( $i % 2 == 1 ) {
        $numbers_array[$i] += $numbers_array[$i];
        if ( $numbers_array[$i] > 9 ) {
            $numbers_array[$i] -= 9;
        }

        echo "<b>" . $numbers_array[$i] . "</b>";
    } else {
        echo $numbers_array[$i]
    }

    $total += $numbers_array[$i];

    # You're doing the same calculation twice (as above). Why not just echo it up there?
    /*if ( $i % 2 == 1 ){
        echo "<b>" . $numbers_array[$i] . "</b>";
    } else {
        echo $numbers_array[$i];
    }*/
}

echo "<br>";
if ( $total % 10 == 0 ) {
    echo "This credit card is valid";
} else {
    echo "This credit card is not valid";
}
<?php

# Grab the numbers array and reverse it. 
$numbers_array = array_reverse( $_POST['array'] );

echo "<br> Credit card Before :- " . implode("", $numbers_array) . 
     "<br><br> Credit card after applaying mod 10 Algorthim :- ";

# Start converting the number
$size = count($numbers_array);
for ( $i = 0; $i < $size; $i++ ) {
    if ( $i % 2 == 1 ) {
        $numbers_array[$i] += $numbers_array[$i];
        if ( $numbers_array[$i] > 9 ) {
            $numbers_array[$i] -= 9;
        }

        echo "<b>" . $numbers_array[$i] . "</b>";
    } else {
        echo $numbers_array[$i]
    }

    $total += $numbers_array[$i];
}

if ( $total % 10 == 0 ) {
    echo "<br>This credit card is valid";
} else {
    echo "<br>This credit card is not valid";
}

Context

StackExchange Code Review Q#44731, answer score: 2

Revisions (0)

No revisions yet.