CODEWITHSUNDEEP
php
i-CONICA
i-CONICA

Reputation: 2379

How to improve this nested PHP if/else structure?

I've written some code, it might not even run, not tried yet. But I've written it all with if else pairs where needed. I can't think of a better way of writing it. Is there a better way that's obvious to you?

The code is going to go into the head of the script and intercept the building of the page if a form with the field "email_notification" has been submitted. If so, it does it's thing, checking if they've entered an email address, and if the item they're subscribing to exists, then adds them, giving error status messages with each else statement. Eventually, it'll redirect to another page with the result of what's happened and kill execution of the rest of the script.

Thanks.

if (isset($_POST['email_notification'])) {
if (!empty($_POST['software_id'])) {
    if (!empty($_POST['email_address'])) {
        if ($result = mysql_query("SELECT shortname, fullname FROM soft_data WHERE shortname = '{$_POST['software_id']}'")) {
            $fullname = mysql_fetch_array($result);
            if (false !== mysql_query("INSERT INTO email_subs(soft_name, email_address) VALUES('{$_POST['software_id']}', '{$_POST['email_address']}')")) {
                $status = "Your email address will be notified when " . $fullname['fullname'];
            }
        } else {
            $status = "Software ID not found.";
        }
    } else {
        $status = "No email address entered.";
    }
} else {
    $status = "No software ID entered.";
}
header("Location: http://example.com/subscription/?status=$status");
die();
}

Upvotes: 0

Views: 258

Answers (3)

Darko Miletic
Darko Miletic

Reputation: 1356

You can also do something like this for quick variable presence check:

function reportStatus($status) {
    header("Location: http://example.com/subscription/?status=$status");
    exit();
}

$ar = array(
    'email_notification' => null,
    'software_id'        => 'No software ID entered.',
    'email_address'      => 'No email address entered.'
);

$df = array_diff($ar, $_POST);

if (!empty($df)) {
    $status = current($df);
    if (!empty($status)) {
        report_status($status);
    }
}

And than continue with your code...

Upvotes: 0

Rudu
Rudu

Reputation: 15892

I'd either embed the lot in a method and return the status (string) or true (if successful), or build a helper-method that outputs the status and ends the script like following:

<?php
function reportStatus($status) {
    header("Location: http://example.com/subscription/?status=$status");
    exit();
}

if (!isset($_POST['email_notification']))
    reportStatus("No email address entered.");
if (empty($_POST['software_id']))
    reportStatus("No software ID entered.";
if (empty($_POST['email_address']))
    reportStatus("No email address entered.");
if (false === $result = mysql_query("SELECT shortname, fullname FROM soft_data WHERE shortname = '{$_POST['software_id']}'"))
    reportStatus("Software ID not found.");
$fullname = mysql_fetch_array($result);
if (false !== mysql_query("INSERT INTO email_subs(soft_name, email_address) VALUES('{$_POST['software_id']}', '{$_POST['email_address']}')"))
    reportStatus("Your email address will be notified when " . $fullname['fullname']);
    //Is above actual an error??
/* Rest of script processing goes here*/
?>

Upvotes: 0

soju
soju

Reputation: 25312

For example :

$errors = array();
if (empty($_POST['software_id'])) $errors[] = 'No software ID entered.';
if (empty($_POST['email_address'])) $errors[] = 'No email address entered.';
...
if (count($errors)) {
  // display errors
} else {
  // process
}

Upvotes: 1

Related Questions