Adding credit cards to a database

Question

I am fairly new to PHP, and would love to have my code reviewed to see what I am doing wrong and ways to improve it. The code works fine, I just feel like there's an easier way to do this.

This is the PHP code at the top of my page - and then the HTML is below it, but I will only paste the PHP code. What the page is, is a way for a shop to edit which credit cards they accept.

Instead of a form, I have the credit card logos on the page and if it's selected in the database then it is highlighted at 100% opacity, and if it's not selected then it's at 25% opacity, so it's faded out. If they choose to start accepting Mastercard for example, they click on the faded Mastercard logo and it goes to ?card=mastercard&value=1 and it updates the database and refreshes the page, so when it refreshes it is now full opacity and selected in the database.

In the database it is called "credit_cards" and the values are "xxxx" for "visa, mastercard, discover, amex"

So if they are all selected, it will show "1111" if all except discover then it is "1101"

<?php
require_once('MySqlDb.php');
$Db = new MySqliDb('localhost', 'root', 'root', 'shops');

if(!isset($_GET['user_id'])) {
die("No user selected.");
}
$id = $_GET['user_id'];

$Db->where('id', $id);
$result = $Db->get('users');

if(isset($_GET['card']) && isset($_GET['value'])) {

if ($_GET['value'] == "1" || $_GET['value'] == "0") {

$card = $_GET['card'];
$value = $_GET['value'];

// Get Current Credit Cards Value In Database
$currentCreditCards = $result[0]['credit_cards'];

// Individual Credit Card Values
$visa = substr($currentCreditCards, 0, 1);
$mastercard = substr($currentCreditCards, 1, 1);
$discover = substr($currentCreditCards, 2, 1);
$amex = substr($currentCreditCards, 3, 1);

switch($card) {
    case "visa":
        // Get The New Value To Update The Database With
        $newCreditCards = $value ."". $mastercard ."". $discover ."". $amex;

        $updateData = array('credit_cards' => $newCreditCards);

        $Db->where('id', $id);
        $Db->update('users', $updateData);
        header("location: test.php?user_id=$id");
    break;

    case "mastercard":
        $newCreditCards = $visa ."". $value ."". $discover ."". $amex;

        $updateData = array('credit_cards' => $newCreditCards);

        $Db->where('id', $id);
        $Db->update('users', $updateData);
        header("location: test.php?user_id=$id");
    break;

    case "discover":
        $newCreditCards = $visa ."". $mastercard ."". $value ."". $amex;

        $updateData = array('credit_cards' => $newCreditCards);

        $Db->where('id', $id);
        $Db->update('users', $updateData);
        header("location: test.php?user_id=$id");
    break;

    case "amex":
        $newCreditCards = $visa ."". $mastercard ."". $discover ."". $value;

        $updateData = array('credit_cards' => $newCreditCards);

        $Db->where('id', $id);
        $Db->update('users', $updateData);
        header("location: test.php?user_id=$id");
    break;
}

}
}

foreach($result as $row) :

if($row['credit_cards'][0] == "1"){ 
    $visaNumber = "0";
    $visaStyle = "style=\"opacity: 1; filter: alpha(opacity=100)\""; 
} else { 
    $visaNumber = "1";
    $visaStyle = "style=\"opacity: 0.25; filter: alpha(opacity=25)\"";
}
if($row['credit_cards'][1] == "1"){ 
    $masterCardNumber = "0";
    $masterCardStyle = "style=\"opacity: 1; filter: alpha(opacity=100)\""; 
} else { 
    $masterCardNumber = "1";
    $masterCardStyle = "style=\"opacity: 0.25; filter: alpha(opacity=25)\"";
}
if($row['credit_cards'][2] == "1"){ 
    $discoverNumber = "0";
    $discoverStyle = "style=\"opacity: 1; filter: alpha(opacity=100)\""; 
} else { 
    $discoverNumber = "1";
    $discoverStyle = "style=\"opacity: 0.25; filter: alpha(opacity=25)\"";
}
if($row['credit_cards'][3] == "1"){ 
    $amexNumber = "0";
    $amexStyle = "style=\"opacity: 1; filter: alpha(opacity=100)\""; 
} else { 
    $amexNumber = "1";
    $amexStyle = "style=\"opacity: 0.25; filter: alpha(opacity=25)\"";
}
?>

Is this a stupid way of doing it? Am I doing this wrong?

The whole visaNumber and visaStyle stuff is just for the HTML, so if its selected in the database, then the URL needs to point to ?visa=0 to turn it off, instead of ?visa=1, and then it has to show the correct opacity.

Answer 1

The first part that updates the selected credit card has a lot of code duplication that could be made generic.

// Get Current Credit Cards Value In Database
$userCards= $result[0]['credit_cards'];

$CARDS = array('visa', 'mastercard', 'discover', 'amex');
$cardIndex = array_search($card, $CARDS);
if ($cardIndex !== false) {
    $userCards[$cardIndex] = $value;
    $updateData = array('credit_cards' => $userCards);
    $Db->where('id', $id);
    $Db->update('users', $updateData);
    header("location: test.php?user_id=$id");
}

Update: Start by replacing named variables such as $visaNumber and $visaStyle with an array of number/styles so these become $cardNumber[0] and $cardStyle[0]. Add another loop to assign the numbers and styles.

$cardNumbers = array();
$cardStyles = array();
foreach ($result as $row) {
    foreach ($CARDS as $cardIndex => $cardName) {
        if ($row['credit_cards'][$cardIndex] == "1") {
            $cardNumbers[$cardIndex] = "0";
            $cardStyles[$cardIndex] = 'style="opacity: 1; filter: alpha(opacity=100)"';
        } else {
            $cardNumbers[$cardIndex] = "1";
            $cardStyles[$cardIndex] = 'style="opacity: 0.25; filter: alpha(opacity=25)"';
        }
    }
}

Without seeing how the numbers and styles are used, I may have gotten the above wrong. What exactly does $result hold? What is in each $row?

Answer 2

Why do you store credit cards data in the database ? What is the purpose of this information?

You should use CSS classes in your code:

.deselected {
  opacity: 0.25; filter: alpha(opacity=25)
}

The selected cards need no additional style, HTML elements are already fully opaque.