3
\$\begingroup\$

I have a page where the user can either input a start date and an end date into two date fields, OR the user can select 10 days, 20 days, 30 days from a drop down list on the page. My php/html knowledge is very basic and I have the syntax set-up like such which works, but there is probably a more efficient way to code this syntax. Speed is my #1 priority here (and this current syntax will have the page load in roughly 3 seconds, sweet!) - but is there optimization/better logic flow that can be used for htis?

<br>Start Date:<input type="date" name="bgin" value="<?= $_POST['bgin'] ?>">
End Date:<input type="date" name="end" value="<?= $_POST['end'] ?>">
<p>
    Show last 10 - 20 - 30 Days
    <select name="daterange">
        <option value="">Select...</option>
        <option value="10">10 Days</option>
        <option value="20">20 Days</option>
        <option value="30">30 Days</option>
    </select>
</p>
<input type="submit" name="submit" value="Show Me">
<?php
    $bgin = $_POST['bgin'];
    $end   = $_POST['end'];
    if (isset($_POST['daterange'])) {
        $datecriteria = $_POST['daterange'];
    }
    //The user input a begin and end date so process based off those values
    if (isset($bgin) && isset($end)) {
        //Connect to mssql here
        $db                 = JDatabase::getInstance($option);
        $query              = $db->getquery(true);
        $query              = "Select statement blah blah where datefield >= '$bgin' and datefield <= '$end'";
    }
    if (isset($datecriteria)) {
        //Connect to mssql here
        $db                 = JDatabase::getInstance($option);
        $query              = $db->getquery(true);
        $query              = "Select statement blah blah
        if ($datecriteria = '10') {
            $query              = " where datefield >= DateAdd(day, -10, getdate()) And datefield <= getdate())";
        }
        if ($datecriteria = '20') {
            $query              = " where datefield >= DateAdd(day, -20, getdate()) And datefield <= getdate())";
        }
        if ($datecriteria = '30') {
            $query              = " where datefield >= DateAdd(day, -30, getdate()) And datefield <= getdate())";
        }
    }   
    else {
        echo "Please check the selection criteria and process again."
    }
?>
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I'm not familiar with Joomla's database library, so forgive me if some details below are suboptimal. But this is how it looks like you could start cleaning the script up.

// Query the dates.
$records = QueryDates();

// If we didn't get any dates...
if($records == null)
{
    echo "Please check the selection criteria and process again.";
}
else
{
    // Do something.
}



// Put your logic in a function that can be called wherever you need.
// You'll eventually want your PHP logic to be in a different file from
// your presentation markup.
function QueryDates()
{
    $bgin = $_POST['bgin'];
    $end  = $_POST['end'];

    if (isset($_POST['daterange'])) 
    {
        $datecriteria = $_POST['daterange'];
    }

    // Wrap our database interactions in a try-catch for safety.
    //
    // You might want to comment out the try-catch when first debugging.
    try
    {
        // Connect to the database.
        //
        // Is $option defined somewhere?
        $db = JDatabase::getInstance($option);

        // Get a new query object.
        $query = $db->getquery(true);

        // SELECT * FROM table_name ...
        $query->select('*');
        $query->from($db->quoteName('table_name'));

        // Get a quoted version of our datefield column.
        $datefield_name = $db->quoteName('datefield');

        if (isset($bgin) && isset($end)) 
        {
            $query->where("$datefield_name >= " . $db->quote($bgin), 'AND');
            $query->where("$datefield_name <= " . $db->quote($end));
        }
        elseif (isset($datecriteria)) 
        {
            // Make sure the data is well-formed.
            if ($datecriteria != 10 
                && $datecriteria != 20 
                && $datecriteria != 30)
            {
                return null;
            }

            // Calculate our range.
            $min_date = DateAdd(day, $datecriteria * -1, getdate());

            $query->where("$datefield_name >= " . $db->quote($min_date), 'AND');
            $query->where("$datefield_name <= " . $db->quote(getdate()));
        }   
        else 
        {
            return null;
        }

        // Execute the query.
        $db->setQuery($query);
        $results = $db->loadObjectList();

        return $results;
    }
    catch (Exception $e)
    {
        // Could return something more useful, based on what 
        // is containe din $e.
        return null;
    }
}

You'll notice a few things here. Let's break them down:

  1. I put your logic into a function. It's a good habit to get into, as you start to learn more about PHP and web development. You'll start to out-grow the mode of putting all your PHP code in the same file as your HTML markup and will start to write functions you want to call from multiple pages.

  2. I changed your query from being a string to a series of function calls. Joomla appears to utilize an abstraction library where you build queries out of function calls against a query object. It also appears to use quoteName for input sanitation.

    That last part sounds iffy, to me. If Joomla has query parameters, I'd recommend you use those, instead. But, as in the intro, I'm not familiar with the Joomla db library so this is what their documentation seems to suggest doing. https://docs.joomla.org/Selecting_data_using_JDatabase

    In either case, don't simply pass a POST parameter into the query. You'll be opening up your entire database to anyone who knows basic SQL Injection (including the droves of SQL Injection bots that will find your site).

  3. I have the new function return null whenever something goes wrong. This allows you to check for the bad output after the function call and handle it there, however you need to. Again, if you were to call this function from different pages, each might have its own way of dealing with the bad output.

    The Joomla documentation doesn't say if loadObjectList() returns something like null or false if the query fails. If it does return null, you might need to change all my "return null;"s to be something like "return false;", so you don't confuse the two outputs.

  4. Finally, I set a try-catch block around your database interaction. This is good practice in production, as it will avoid nasty errors from hitting your users. But it may be troublesome while you're developing the code (as it will prevent you from seeing when something goes wrong). I'd recommend commenting out the try-catch until you do publish the site to your production environment.


As for speed, most of those 3 seconds for loading is likely network flight time (though, I assume you're running this locally) and the interaction with your database. There's not a lot you can do to speed up the traffic of sending the query and receiving the data, outside of environmental changes (eg. getting a faster hard drive on the server).

The one thing you can do is make sure your database schema is structured to facilitate speedy queries. For instance, if you're constantly querying against this "datefield", you might consider putting an index on it.

Depending on the data being returned, you can also tweak the cache. For instance, if users are frequently querying the same data, ensure that the cache is actually storing that data (rather than saying it's too big and dumping it or having the cache outright turned off).

If you expect a bunch of users to be using the page, you might also want to put stricter limits on the range of dates that can be queried. Having someone try to dump your entire database could cause slowdowns for other users.

But a lot of this is up to details you haven't given us.

\$\endgroup\$
2
  • \$\begingroup\$ the where clauses are never being added to the query string - even if I verify that isset = true. I have echo on screen and never is the where added. Any idea why? \$\endgroup\$ Commented Jul 11, 2017 at 15:35
  • \$\begingroup\$ That's a bit odd but, like I said, I'm not familiar with JDatabase. So my syntax might be wrong. But it looks just like how others have it. Have you tried placing a $query->where('stuff'); just beneath the $query->from('table_name');? \$\endgroup\$
    – Siegen
    Commented Jul 11, 2017 at 16:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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