patternphpMinor
Credit card validation with reversed array
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:
With no comments:
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.