3
\$\begingroup\$

Is the method used below to get data from a database efficient and optimal? The data is in a MySQL database, and in my server I have a PHP file with the following code to return some information:

if($_POST["method"] == "requestBusinessFood") {
    requestBusinessFood();
} 

function requestBusinessFood() {
    $categoryID = $_POST["category"];

    $host = 'localhost';
    $user = 'root';
    $pass = '';
    $db = 'fooddatabase';

    $conn = mysqli_connect($host, $user, $pass, $db); 
        $sql = "SELECT * FROM `foodtablebusiness` WHERE category = " . $categoryID;

        $result = mysqli_query($conn, $sql);
        $rows = array();
        while($r = mysqli_fetch_assoc($result)) {
         $rows[] = $r;
        }

        echo json_encode($rows);
}

On the webpage, I have a js file to retrieve the information in the following way:

  function createAJAXRequestToPopulateList(category) {
    return $.ajax({ url: '../server.php',
                    data: {method: 'requestBusinessFood',
                           category: category},
                    type: 'post'
    });
  }

function addActivityItem(){
    var selector = document.getElementById("categorySelector");
    ajaxRequest = createAJAXRequestToPopulateList(selector.options[selector.selectedIndex].value);
    ajaxRequest.done(populateList);
}

function populateList(responseData) {
    console.log(responseData);
}
\$\endgroup\$
1
  • \$\begingroup\$ Use mysqli_real_escape_string. \$\endgroup\$ May 28 '20 at 6:34
2
\$\begingroup\$

PHP

SQL injection vulnerabilities

This code is wide-open to SQL injection attacks. User input should be sanitized as supplied to the query using parameters (e.g. with mysqli_prepare() and bind_param()).

Indentation

The indentation is somewhat consistent but then in some places it increases without a block-level change - e.g. in the PHP code most lines are indented with four spaces, which is very common, and then when $sql is declared it increases to eight spaces.

$conn = mysqli_connect($host, $user, $pass, $db); 
      $sql = "SELECT * FROM `foodtablebusiness` WHERE category = " . $categoryID; 

Fetching results

Instead of using a while loop with mysqli_fetch_assoc() just to push into an array, use mysqli_fetch_all() to get an array with one call.

Selecting fields

Additionally, the SQL query selects all fields - i.e. with SELECT *. Instead of selecting *, specify the field names needed in order to exclude any fields not needed in the front end.

Javascript

Variable scope

In this line:

ajaxRequest = createAJAXRequestToPopulateList(selector.options[selector.selectedIndex].value);

it makes a global variable ajaxRequest because there is no var, let or const keyword before it. Use const to limit the scope to the function addActivityItem.

Utilizing jQuery

It appears that jQuery is used, given the usage of $.ajax() (though if that is wrong then the following won't work). Presuming that is the case, then the other code can be simplified using jQuery utilities.

For example:

var selector = document.getElementById("categorySelector");
ajaxRequest = createAJAXRequestToPopulateList(selector.options[selector.selectedIndex].value);

can be simplified to the following using the .val() method:

const selector = $('#categorySelector');
const ajaxRequest = createAJAXRequestToPopulateList(selector.val());
\$\endgroup\$
2
\$\begingroup\$

Sadly, the PHP part is neither efficient nor optimal.

Apart from already mentioned issues, your function does too much work, which makes it non-reusable and which bloats your code in general. Remember the rule of thumb: each unit of code should mind its own business. While your function interacts with a client through $_POST and echo, connects to a database, queries a database, formats the results. You have split it into several parts

  1. Create a separate file to connect with a database and just include it in every script that needs a database connection.
  2. Create a function that performs a certain SQL query and return the results, so it can be reused for any other kind of request.
  3. Write a code that handles a particular request from a client

So the code should be

<?php
require 'mysqli.php';

if($_POST["method"] == "requestBusinessFood")
{
    $rows = requestBusinessFood($mysqli, $_POST["category"]);
    echo json_encode($rows);
} 

function requestBusinessFood($mysqli, $categoryID)
{
    $sql = "SELECT * FROM foodtablebusiness WHERE category = ?";
    $stmt = $mysqli->prepare($sql);
    $stmt->bind_param("s", $categoryID);
    $stmt->execute();
    $result = $stmt->get_result();
    return $result->fetch_all(MYSQLI_ASSOC);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.