GrumpyCrouton
GrumpyCrouton

Reputation: 8621

Multiple buttons on multiple pages that do ALMOST the same thing

I need some advice on my script. I am developing a roll call system, and I want to get out of a bad habit of mine.

Here is a pic of what I want: https://i.sstatic.net/UdqMQ.png

Specifically, the buttons above each of the day columns. Each of these buttons do nearly the same thing, but for different dates in my SQL database.

So here is the code I have:

Inputs:

<th><input type='submit' id='mAllR' name='mAllR' value='All Attended'></th>
<th><input type='submit' id='tAllR' name='tAllR' value='All Attended'></th>
<th><input type='submit' id='wAllR' name='wAllR' value='All Attended'></th>
<th><input type='submit' id='tAllR' name='tAllR' value='All Attended'></th>
<th><input type='submit' id='fAllR' name='fAllR' value='All Attended'></th>

As you can see, its basically each button having its own ID which will call a function by its name.

This would be fine by me if there were only 5 input buttons, which there are in my example, but there are 7 departments, which means 35 buttons. I do not want to make 35 functions that do ALMOST the same thing.

The code I have to work with that I can adapt is below:

if(isset($_POST['mAllR']))
            {
                    $index      = 0;
                    foreach($_POST as $key => $value)
                            {
                                    while($index < $_SESSION['amountT'])
                                            {
                                                    $index++;
                                                    $sql = ("UPDATE `Employee` SET `Monday`='Attended' WHERE `Job`='Remarketing'");
                                                    mysqli_query($con, $sql);
                                            }
                            }
                    if($sql)
                            {
                                    mysqli_close($con);
                                    sleep(4);
                                    echo '<meta http-equiv="refresh" content="0">';
                            }
            }

Each button, the way it is now, will needs its own version of this. (Do not worry about the sessions ETC, those are handled elsewhere to avoid overlapping/underlapping.

So really, the code is not the issue, I just want to optimize this into less code. Any suggestions are welcome.

Upvotes: 2

Views: 79

Answers (2)

CyberPunkCodes
CyberPunkCodes

Reputation: 3777

I would use better name values for your buttons. For example: day-who-job, so it would be like: mon-all-remarketing. Then explode the value to get the day, who, and job. I am going to also add "sub" to the name, so we can easily find it later. It just lets us know its a submit button. Since you don't show any other items in your form, I am passing a hidden field so we can tell if it's been submitted.

<form action="" method="POST">
<input type="hidden" name="myform" value="1" />
<th><input type='submit' id='mon-all-remarketing' name='sub-Monday-All-Remarketing' value='All Attended'></th>
<th><input type='submit' id='tue-all-remarketing' name='sub-Tuesday-All-Remarketing' value='All Attended'></th>
<th><input type='submit' id='wed-all-remarketing' name='sub-Wednesday-All-Remarketing' value='All Attended'></th>
<th><input type='submit' id='thu-all-remarketing' name='sub-Thursday-All-Remarketing' value='All Attended'></th>
<th><input type='submit' id='fri-all-remarketing' name='sub-Friday-All-Remarketing' value='All Attended'></th>
</form>

Now the name of the clicked button will be passed via POST when the form is submitted.

<?php
function updateAttendance($day, $who, $job) {
    $sql = ("UPDATE `Employee` SET `$day`='Attended' WHERE `Job`='$job'");
    if ($result = mysqli_query($con, $sql)) {
        echo '<meta http-equiv="refresh" content="0">';
    }
}

if ( isset($_POST['myform']) && ($_POST['myform'] == 1) ) {
    // forms been submitted
    foreach ($_POST as $key => $val) {
        if( strstr($key, 'sub-') ) {
            // if found, it's your button. ex: sub-Tuesday-All-Remarketing
            $array = explode('-', $key);  // now its split by the dash "-"
            $day = $array[1];  // Tuesday
            $who = $array[2];  // All
            $job = $array[3];  // Remarketing
            updateAttendance($day, $who, $job);
        }
    }
}

There might be some flaws in that, but I hope you get where I was going with it. You would want to make sure your queries are sanitized though, using data from the page in an SQL query is not safe. If your not using prepared statements, then you need to sanitize them, or put them through a switch case statement, and use the results from the switch case..

This would do two things, shorten your submit button by using abbreviated days, and would be safe for use in an SQL query:

<?php
function returnDay($day) {
    switch($day) {
        case "mon":
            return "Monday";
            break;
        case "tue":
            return "Tuesday";
            break;
        // etc etc
    }
    return false;
}

You could use that function in the "updateAttendance()" function, to be sure it is safe data. You would have to name your buttons with the shorter names: sub-mon-all-remarketing.

You could also, duplicate this switch case function to handle "Remarketing", or any other value.

Upvotes: 1

Eric Martinez
Eric Martinez

Reputation: 31777

Since all of your buttons do almost the same thing, I'm guessing in each case just change some value in the query.

I would do it like this

First I would create an array containing the different data for each button (day and job since this is the only difference I can see the code needs)

// Monday
$array["mAllR"]["day"] = "Monday";
$array["mAllR"]["job"] = "Marketing";
// Tuesday
$array["tAllR"]["day"] = "Tuesday";
$array["tAllR"]["job"] = "Aonther job";
// Etc...

Then in the foreach loop I'd check for the button's name (see that in the array the main key is the name button)

foreach($_POST as $key => $value) {
    $button = "";
    // I check for the value of the button to get the key (button's name)
    if($value == 'All attended') {
        $button = $key;

        // Then I call the 'day' and 'job' values from the array
        // where the main key is the button's name
        while($index < $_SESSION['amountT'])
        {
            $index++;

            // See that I pass the day and job  defined in the array using the button's name
            $sql = ("UPDATE `Employee` SET `"+$array[$button]["day"]+"`='Attended' WHERE `Job`='"+$array[$button]["job"]+"'");
            mysqli_query($con, $sql);
        }
    }
}

Edit : Wrap it inside a function.

// Wrap
function postFunc() {
    // The code above
}

// Use
if(isset($_POST)) {
    postFunc();
}

That would be my approach. But it has a cons, since I'm checking for the value of the button and doing everything inside that condition I lose control over every other field you might want to control. I don't see in the code you're trying to control another field, so this approach might be useful.

Upvotes: 2

Related Questions