Code works until I make it a function

Question

I am working on a PHP file that contains all my functions. I am currently sending the info through GET, then the file determines what action I am trying to do. Here is how my file is currently.

if ($action == 'creditBal')
{
    $user_id = $_GET['user_id'];
    $amt = $_GET['amt'];

    $mysql_query = "UPDATE Accounts SET BALANCE=BALANCE+$amt WHERE ID=$user_id";
    $result = mysql_query($mysql_query, $mysql_conn);

    if (!$result)
    {
        # die('MySQL Error: ' . mysql_error());

        $json_array = array(
            'response' => 'mysql_error'
        );

        return;
    }

    $json_array = array(
        'response' => 'success',
    );

    echo json_encode($json_array);

    mysql_close($mysql_conn);
}

The code above works great until I decide to change it up to the following.

function creditBal($user_id, $amt)
{
    $mysql_query = "UPDATE Accounts SET BALANCE=BALANCE+$amt WHERE ID=$user_id";
    $result = mysql_query($mysql_query, $mysql_conn);

    if (!$result)
    {
        # die('MySQL Error: ' . mysql_error());

        $json_array = array(
            'response' => 'mysql_error'
        );

        return;
    }

    $json_array = array(
        'response' => 'success',
    );

    return json_encode($json_array);

    mysql_close($mysql_conn);
}

if ($action == 'creditBal')
{
    $user_id = $_GET['user_id'];
    $amt = $_GET['amt'];

    echo creditBal($user_id, $amt);
}

I've tried checking for misspellings and everything, but it always doesn't work. Once I make it the if $action equals only, then it works. For some reason, the function doesn't work.

This has me really puzzled.

[Please, don't use `mysql_*` functions in new code](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). See the [red box](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://j.mp/PoWehJ). **You are also wide open to [SQL injections](http://stackoverflow.com/q/60174)** — John Conde, May 12 '14 at 23:58
`global $mysql_conn;` to start with, and when your `$result` is false you should `return json_encode($json_array);` instead of returning nothing. This is likely to be why your function isn't working - because your `mysql_connect` is failing, and your error handling of that is simply to return nothing. — scrowler, May 12 '14 at 23:58
@JohnConde I know that this is prone to injection. I am just working on the basics first then getting into that later. — Alec, May 12 '14 at 23:59
@Alec You need to read the rest of his comment, the `mysql_*` functions are deprecated and will be removed in a future version of PHP. They are no longer being actively maintained and you should not be learning with them anymore. — Alex.Ritna, May 13 '14 at 00:02
@Alec That or PDO, personally I prefer PDO but if you're already using the `mysql_*` functions, the transition to [MySQLi](http://php.net/mysqli) is going to be a little simpler. Check out the quick start guide [here](http://php.net/mysqli). — Alex.Ritna, May 13 '14 at 00:09

score 6 · Accepted Answer · answered May 13 '14 at 00:01

6

Two things:

You don't have $mysql_conn available to your function. You can pass it as a parameter or use the global keyword to make it available in your function.
When there is a MySQL error you don't actually return a response. You return nothing.
As I mentioned in my comments, this code is insecure and vulnerable. Please fix that, too.

.

function creditBal($user_id, $amt, $mysql_conn)
{
    $mysql_query = "UPDATE Accounts SET BALANCE=BALANCE+$amt WHERE ID=$user_id";
    $result = mysql_query($mysql_query, $mysql_conn);

    if (!$result)
    {
        # die('MySQL Error: ' . mysql_error());

        $json_array = array(
            'response' => 'mysql_error'
        );
    }
    else {
        $json_array = array(
            'response' => 'success',
        );
    }
    mysql_close($mysql_conn);
    return json_encode($json_array);

}

if ($action == 'creditBal')
{
    $user_id = $_GET['user_id'];
    $amt = $_GET['amt'];

    echo creditBal($user_id, $amt, $mysql_conn);
}

answered May 13 '14 at 00:01

John Conde

217,595
99
455
496

That 94k's gonna hit soon ;-) – Funk Forty Niner May 13 '14 at 00:02
Thank you John. I blanked on this one... stupid mistake. I left out the global $mysql_conn. I know that my json response is blank too. – Alec May 13 '14 at 00:04
John I know it is very vulnerable. I'm focusing more on getting a working model first, then security etc. later on. – Alec May 13 '14 at 00:05
1

@Alec As long as it gets done. FYI, it's almost always easier to do it from the get go. – John Conde May 13 '14 at 00:06
@JohnConde I guess you're right. Should I use MySQLi? – Alec May 13 '14 at 00:07
@Alec That would be the easiest to start with since it is very similar to mysql_*. – John Conde May 13 '14 at 00:08
@Alec The `i` in `mysqli` stands for "improved" so, yeah, you can say that. :) – John Conde May 13 '14 at 00:10
@JohnConde Haha ok that pretty much says it! Thanks John! – Alec May 13 '14 at 00:12

Code works until I make it a function

1 Answers1