Else condition won't run

Question

I know the title is very annoying but anyways I start my question. I have created a function in a class that's kinda a multi-purpose one. I gave it a parameter which is of boolean type and of course only accepts "true" and "false" as values. Next, I am calling out that function through another PHP file on which the data is posted through ajax and that ajax is called through a button on my HTML page. Let me demonstrate it as a diagram:

flow chart showing my program flow

I hope you've understood my program flow, now let's come to the code or in other words here is what I've tried:

HTML Page's button:

document.write('<span class="pull-right" style="margin-top: -30px; margin-right: 20px;"><button id="accept_offer" class="btn btn-success" onclick="setPostOfferAcceptance(32);">Accept Offer</button></span>');

AJAX Page:

<?php

require_once '../../Classes/MyClass.php';

$offer_id = $_POST["id"];
$acceptance = $_POST["acceptance"];

$accepted = MyClass::SetOfferAcceptance($offer_id, $acceptance);

if($accepted) $data["status"] = "success";
else $data["status"] = "error";

echo json_encode($data);

?>

MyClass's Function:

public static function SetOfferAcceptance($offerId, $acceptance) {
    if(Utilities::IsValid($offerId) && Utilities::IsValid($acceptance)) {
        Database::OpenConnection();

        $offerId = Utilities::SafeString(Database::$databaseConnection, $offerId);
        $acceptance = Utilities::SafeString(Database::$databaseConnection, $acceptance);

        $query = "";
        if($acceptance == true) {
            $query = Database::$databaseConnection->prepare("UPDATE post_offer SET Accepted=1 WHERE Offer_ID = ?");
        } else {
            $query = Database::$databaseConnection->prepare("UPDATE post_offer SET Accepted=0 WHERE Offer_ID = ?");
        }

        $query->bind_param("i", $offerId);
        $result = $query->execute();
        Database::CloseConnection();
        if($result) return 1; else return -1;
    }
}

And sorry, but finally, here's my javascript function that posts the data to the AJAX page:

function setPostOfferAcceptance(offer_id) {
    if($("#accept_offer").text() == "Accept Offer") {
        $.post("admin/post_offer/set_post_offer_acceptance.php", {id: offer_id, acceptance: true}, function(data){
            if(data.status == "success") {
                $("#acceptedOfferModal").modal("show");
                setTimeout(function(){ $("#acceptedOfferModal").modal("hide"); }, 2500);
                $("#accept_offer").text("Unaccept Offer");
                console.log(data.status);
            }
        }, 'json');
    } else if($("#accept_offer").text() == "Unaccept Offer") {
        $.post("admin/post_offer/set_post_offer_acceptance.php", {id: offer_id, acceptance: false}, function(data){
            if(data.status == "success") {
                $("#unacceptedOfferModal").modal("show");
                setTimeout(function(){ $("#unacceptedOfferModal").modal("hide"); }, 2500);
                $("#accept_offer").text("Accept Offer");
                console.log(data.status);
            }
        }, 'json');
    }
}

EDIT: I forgot to post the JS code. But I've updated it now. Please consider it. Thanks.

The problem is that the function SetOfferAcceptance() is never running the else condition which is that if $acceptance is not equal to true.

Why? Can anyone please point out my mistake.

Thanks.

[Don't use `document.write`!](http://www.w3.org/TR/2011/WD-html5-20110525/apis-in-html-documents.html#document.write) — Oriol, Dec 27 '13 at 18:00
@user689 I know he would recommend me to use a templating engine. — Mohammad Areeb Siddiqui, Dec 27 '13 at 18:04
@user1336827 I've posted my JS code which changes the value of acceptance. — Mohammad Areeb Siddiqui, Dec 27 '13 at 18:05
@user689 For example, `document.getElementById('myElement').innerHTML = /* ... */`. — Oriol, Dec 27 '13 at 18:13
@user689 It's better than `document.write`. Of course `document.createElement`, `document.createTextNode`, `.appendChild`, etc. are safer (since HTML injection is not possible), but they don't provide the same functionality than `document.write`. — Oriol, Dec 27 '13 at 18:29
@Oriol Using a [library](http://stackoverflow.com/questions/737307/javascript-is-it-better-to-use-innerhtml-or-lots-of-createelement-calls-to-ad) would. However I was just pointing that the next time when you say "*Don't use ...*". Provide an alternative! — , Dec 27 '13 at 18:39

score 0 · Accepted Answer · edited May 23 '17 at 10:25

0

If acceptance is boolean then use the following if...else, i.e. with tripple equals $acceptance === true:

// In AJAX page
if ($acceptance === true) {
  $query = Database::$databaseConnection->prepare("UPDATE post_offer SET Accepted=1 WHERE Offer_ID = ?");
} else {
  $query = Database::$databaseConnection->prepare("UPDATE post_offer SET Accepted=0 WHERE Offer_ID = ?");
}

Please reference: "How do the equality (== double equals) and identity (=== triple equals) comparison operators differ?"

Update:

Either check for $acceptance value in SetOfferAcceptace, or pass in boolean value to SetOfferAcceptance. Example with latter:

$offer_id = $_POST["id"];
$acceptance = false;
if (isset($_POST["acceptance"]) && $_POST["acceptance"] == 'accepted') {
  $acceptance = true;
}

$accepted = MyClass::SetOfferAcceptance($offer_id, $acceptance);

edited May 23 '17 at 10:25

Community

1
1

answered Dec 27 '13 at 17:58

vee

38,255
7
74
78

Thanks, but now only the second condition is running :/ It's never making it true. – Mohammad Areeb Siddiqui Dec 27 '13 at 18:02
@Arian Then only the first condition runs. :/ – Mohammad Areeb Siddiqui Dec 27 '13 at 18:03
Aren't all post values string by default? Using a type sensitive comparison would never be `===` true. – Jonathan Kuhn Dec 27 '13 at 18:05
@MohammadAreebSiddiqui, can you post the value of `$_POST['acceptance']`? – vee Dec 27 '13 at 18:05
@JonathanKuhn, yes. The part I missed and paid the price for :( – vee Dec 27 '13 at 18:05
@vee Then any alternative? :/ – Mohammad Areeb Siddiqui Dec 27 '13 at 18:07
In your ajax page, I'd check for `acceptance` and pass a boolean instead of the actual value as the actual value is not used in your `SetOfferAcceptance` function. – vee Dec 27 '13 at 18:09
Thanks, but I figured out an alternative that worked. Please check my answer if you want to know what it was. I just turned boolean into a string. :) I got that idea from @JonathanKuhn. – Mohammad Areeb Siddiqui Dec 27 '13 at 18:13
@MohammadAreebSiddiqui, I added an alternative solution. – vee Dec 27 '13 at 18:14
@vee Thanks again, but that's of no use because I know that `$acceptance` have a value when it comes to the page. – Mohammad Areeb Siddiqui Dec 27 '13 at 18:15
@MohammadAreebSiddiqui, that's fine. Yes `$acceptance` has a value which is why only `if` was executing before as all valid strings evaluate to true. I added the `isset` check for safety, the check `$acceptance == 'accepted'` is what should make `$accepted` true. – vee Dec 27 '13 at 18:17
But according to my previous code, I didn't set the value of `acceptance` to a string it was a boolean so is your alternative related to my previous code or the new one? – Mohammad Areeb Siddiqui Dec 27 '13 at 18:21
The `accepted` literal is based on your new code. Basically what I'm trying to show is you can check for posted values and change boolean `@accepted` to either true or false. – vee Dec 27 '13 at 18:23

score 0 · Answer 2 · answered Dec 27 '13 at 18:12

Thanks you all for your answers but I figured out another alternative. That was instead of making $acceptance a boolean I made it a string and converted my JS code into:

function setPostOfferAcceptance(offer_id) {
    if($("#accept_offer").text() == "Accept Offer") {
        $.post("admin/post_offer/set_post_offer_acceptance.php", {id: offer_id, acceptance: "accepted"}, function(data){
            if(data.status == "success") {
                $("#acceptedOfferModal").modal("show");
                setTimeout(function(){ $("#acceptedOfferModal").modal("hide"); }, 2500);
                $("#accept_offer").text("Unaccept Offer");
                console.log(data.status);
            }
        }, 'json');
    } else if($("#accept_offer").text() == "Unaccept Offer") {
        $.post("admin/post_offer/set_post_offer_acceptance.php", {id: offer_id, acceptance: "unaccepted"}, function(data){
            if(data.status == "success") {
                $("#unacceptedOfferModal").modal("show");
                setTimeout(function(){ $("#unacceptedOfferModal").modal("hide"); }, 2500);
                $("#accept_offer").text("Accept Offer");
                console.log(data.status);
            }
        }, 'json');
    }
}

And my class's function into:

public static function SetOfferAcceptance($offerId, $acceptance) {
    if(Utilities::IsValid($offerId) && Utilities::IsValid($acceptance)) {
        Database::OpenConnection();

        $offerId = Utilities::SafeString(Database::$databaseConnection, $offerId);
        $acceptance = Utilities::SafeString(Database::$databaseConnection, $acceptance);

        $query = "";
        if($acceptance == "accepted") {
            $query = Database::$databaseConnection->prepare("UPDATE post_offer SET Accepted=1 WHERE Offer_ID = ?");
        } else {
            $query = Database::$databaseConnection->prepare("UPDATE post_offer SET Accepted=0 WHERE Offer_ID = ?");
        }

        $query->bind_param("i", $offerId);
        $result = $query->execute();
        Database::CloseConnection();
        if($result) return 1; else return -1;
    }
}

And that worked! :)

Thanks again @vee.

score 0 · Answer 3 · answered Dec 27 '13 at 18:19

0

If I am not mistaken your issue stems from automatic type casting.

if($result) return 1; else return -1; // both of these evaluate to boolean true because -1 is an integer

if($result) return 1; else return 0; // evaluates to true/false respectively

if($result) return true; else return false; // evaluates to true/false respectively

The last line is what you should honestly be using.

answered Dec 27 '13 at 18:19

MonkeyZeus

20,375
4
36
77

Thanks, but that answer is not related to my question and there's a reason behing why I used `1` and `-1`. – Mohammad Areeb Siddiqui Dec 27 '13 at 18:20
Well I just hope this issue doesn't bite you in the butt because the `if($accepted)` block will ALWAYS evaluate to "success" the way it is currently written. – MonkeyZeus Dec 27 '13 at 18:27

Else condition won't run

3 Answers3