2
\$\begingroup\$

I recently started a project which has a database of rail cars for a train simulator. Im trying to set up a way to filter the data based upon 4 categories "Car Type", "Road Name", "Pack", and "Area of Origin". I was able to get the dropdown menus to work but they way I did it seems a bit cumbersome. Essentially I have 4 Jquery Ajax scripts one for each dropdown menu each tied to their own specific php file to run the query. Is there a way to consolidate these 4 files into perhaps 1 file that does the same thing? Below is my code.

Index.php (The car directory main page)

<html>
<head>
    <title></title>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<script>
function showType(str) {
    if (str == "") {
        document.getElementById("txtHint").innerHTML = "";
        return;
    } else { 
        if (window.XMLHttpRequest) {
            // code for IE7+, Firefox, Chrome, Opera, Safari
            xmlhttp = new XMLHttpRequest();
        } else {
            // code for IE6, IE5
            xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
        }
        xmlhttp.onreadystatechange = function() {
            if (this.readyState == 4 && this.status == 200) {
                document.getElementById("txtHint").innerHTML = this.responseText;
            }
        };
        xmlhttp.open("GET","type.php?t="+str,true);
        xmlhttp.send();
    }
}
function showRoad(str) {
    if (str == "") {
        document.getElementById("txtHint").innerHTML = "";
        return;
    } else { 
        if (window.XMLHttpRequest) {
            // code for IE7+, Firefox, Chrome, Opera, Safari
            xmlhttp = new XMLHttpRequest();
        } else {
            // code for IE6, IE5
            xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
        }
        xmlhttp.onreadystatechange = function() {
            if (this.readyState == 4 && this.status == 200) {
                document.getElementById("txtHint").innerHTML = this.responseText;
            }
        };
        xmlhttp.open("GET","road.php?r="+str,true);
        xmlhttp.send();
    }
}
function showPack(str) {
    if (str == "") {
        document.getElementById("txtHint").innerHTML = "";
        return;
    } else { 
        if (window.XMLHttpRequest) {
            // code for IE7+, Firefox, Chrome, Opera, Safari
            xmlhttp = new XMLHttpRequest();
        } else {
            // code for IE6, IE5
            xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
        }
        xmlhttp.onreadystatechange = function() {
            if (this.readyState == 4 && this.status == 200) {
                document.getElementById("txtHint").innerHTML = this.responseText;
            }
        };
        xmlhttp.open("GET","pack.php?p="+str,true);
        xmlhttp.send();
    }
}
</script>
<style type="text/css">
#infoPicture{
    width: 200px;
    height: 113px;
    float: left;
}
#infoInfo{
    height: 200px;
    width: auto;
    float: left;
}
</style>
</head>
<body>
    <form>
    <?php

include ("includes/dbh.inc.php");

//$query = "SELECT * FROM categories WHERE catagoryID = $catagoryID AND catagoryName = $catagoryName";
//Type Dropdown
$type = mysqli_query($conn, "SELECT * FROM types");
    echo '<select onchange="showType(this.value)" name"CarTypes">';
    echo '<option>' . 'Car Type' . '</option>';
    while ($row = mysqli_fetch_array($type)) {
    echo '<option value"' . $row['typeName'] .'">' . $row['typeName'] . '</option>';
    }

    echo '</select>';
//Road Dropdown
$road = mysqli_query($conn, "SELECT * FROM roads");
echo '<select onchange="showRoad(this.value)" name"RoadNames">';
    echo '<option>' . 'Road Name' . '</option>';
    while ($row = mysqli_fetch_array($road)) {
    echo '<option value"' . $row['roadName']. '">' . $row['roadName'] . ' ('.$row['reportingMark']. ')</option>';
    }

    echo '</select>';
//Pack Dropdown
$pack = mysqli_query($conn, "SELECT * FROM packs");
echo '<select onchange="showPack(this.value)" name"AddonPack">';
    echo '<option>' . 'Add on Pack' . '</option>';
    while ($row = mysqli_fetch_array($pack)) {
    echo '<option value"' . $row['packID'] .'">' . $row['packName'] . '</option>';
    }

    echo '</select>';
//Origin Dropdown
$origin = mysqli_query($conn, "SELECT * FROM origins");
echo '<select onchange="showOrigin(this.value)" name"Origins">';
    echo '<option>' . 'Area of Origin' . '</option>';
    while ($row = mysqli_fetch_array($origin)) {
    echo '<option value"' . $row['originID'] .'">' . $row['origin'] . '</option>';
    }

    echo '</select>';
    ?>
    <a href="index.php">Show All</a>
</form>

Type.php I only included one of these as all four are essentially the same but with slightly different queries

<html>
<head>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
</head>
<body>

<?php 
include_once 'includes/dbh.inc.php';

if(isset($_GET['t'])){
    $t = $_GET['t'];
    }else{
        $t = "Name not set in the GET method";
    }

$sql="SELECT * FROM rollingstock WHERE stockType = '".$t."'";
$result = mysqli_query($conn, $sql);

echo '<table border="0">';
if ($result-> num_rows > 0) {
    while ($row = $result-> fetch_assoc()) {
        //echo '<div class="listing-picture" style="background-color:#bbb">';
        echo '<tr><td>';
        echo '<img src="images/Coil_2.png"/>';
        echo '</td>';
        echo '<td width ="500">';
        echo $row["stockName"];
        echo '<br>';
        echo $row["stockPack"];
        echo '<br>';
        echo $row["stockLink"];
        echo '<br>';
        echo $row["stockDesc"];
        echo '</td></tr>';

    }
    echo '</table>';
    
}else{
    echo "Nothing Found!";
} 

 ?>
```
\$\endgroup\$
1
  • \$\begingroup\$ Please take the tour and read How to Ask. Your question title is meant to uniquely describe what your script does. I don't see any jquery in your scripts so that tag is inappropriate. We are not meant to help with adding new features (such as converting your plain js to jquery). Please edit. \$\endgroup\$ – mickmackusa Nov 11 '20 at 22:57
2
\$\begingroup\$

These are my recommendations:

  1. Move your inline javascript listeners to where your other javascript processes are.

  2. Move all javascript processes to an external .js file and call it into your index page. This will reduce the code bloat and put similiar code all in one manageable place.

  3. Move all styling declarations to an external .css file. Again, keeping things meaningfully grouped will aid in code readability and maintenance.

  4. Consistently use mysqli's object-oriented syntax. The syntax is more concise than procedural style.

  5. Never directly inject values into query strings. Use prepared statements with bound parameters.

  6. When the intent of building a result set is to instantly pass the data to another layer, it is recommended to simply use fetch_all(). And for that matter, wrap the whole result set in json_encode() and just pass that payload back to your ajax call.

  7. Do not make it the responsibility of the querying file to generate html. Pass the raw data back to javascript and demand that the markup be constructed by javascript. There are clean ways to do this.

  8. There is never a reason to duplicate the text of an <option> tag as the value value of the same tag. In other words, when the value is the same as the text, omit the value declaration. There is absolutely no benefit in the redundancy.

  9. if $_GET['t'] is not declared (or in anyway invalid), don't even bother connecting to the database. Use an early exit so that resources are not wasted. Write something in the javascript that will inform the user of the missing/invalid data.

  10. Once you have the querying file purely returning data payloads as json back to ajax, then it becomes very clear that you only need to pass the table name and the identifying value to the php script. The php script then uses a whitelist/lookup array to validate that the table name supplied is available and provides the correct (hardcoded) column name that should be used in the query of said table. In doing all of this, your php file will only need to write a single, dynamic prepared statement and pass back the encoded result set. The javascript/ajax will already be aware of what select had triggered the event, so it therefore knows what kind of markup should be generated and where it should go.

\$\endgroup\$
1
  • \$\begingroup\$ You might also want to suggest the OP use jQuery for AJX and DOM selection or else ditch jQuery \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 13 '20 at 23:59

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.