Rendering a history list of shows with different options

Question

I have some PHP code I'm working on which contains 2 queries and I need to set extra conditions for 2 out of 3 view options and one less query for 1 out of 3 view options.

Basically, I'm rendering a history list of shows with three options ALL | PRO | UPCOMING. The ALL option should include both queries and have no extra conditions. The PRO option should have an extra condition with both queries. The UPCOMING option should have an extra condition but only one query.

I've never built an if else statement with queries before, but I'm sure there is an easier way to make the code shorter.

This PHP is also within an API that get pulled to the front end. Is there a way to render buttons on the front end to change the query from ALL to PRO or UPCOMING?

 function get_shows($playid) {

    // There are two tables, so we will need to merge the results
    $results = array();

    $count  = ($this->input->get('count')) ? $this->input->get('count') : $this->max_results;
    $offset = ($this->input->get('offset')) ? $this->input->get('offset') : 0;
    $view = 'all';

    // ALL
    if ($view == 'all') {
        $extra_condition = ""; //If ALL no extra condition needs to be set

    $query = $this->db->query("SELECT distinct customer.customerid as customerid, organization, vcity, vstate, vcountry, firstdate, lastdate 
                               FROM customer, orders, venue 
                               WHERE orders.ordersid=venue.ordersid 
                               AND orders.playid=? AND orders.active=1 
                               AND customer.customerid=orders.customerid 
                               AND shipped != 0 
                               AND defunct != 1 
                               AND orders.lastdate != 0 
                               AND (ordertype != 'misc' 
                               AND ordertype != 'reading') 
                               AND hidepic!=1 
                               AND hideprod!=1 
                               AND num_perf>0 
                               ORDER BY firstdate desc 
                               LIMIT $offset,$count", $playid);
    foreach ($query->result() as $row)
    {
        $results[] = array 
        (
    'id' => $row->customerid,
    'src' => 'customerid',
            'organization' => $row->organization,
            'city' => $row->vcity,
            'state' => $row->vstate,
            'country' => $row->vcountry,
            'firstdate' => call_user_func($fmt_date, $row->firstdate),
            'lastdate' => call_user_func($fmt_date, $row->lastdate)
        );
    }

    // Legacy data from oldprods
    $query = $this->db->query("SELECT oldprodsid, organization, city, state, country, firstdate, lastdate  
                               FROM oldprods where playid=? 
                               ORDER BY firstdate desc 
                               LIMIT $offset,$count",$playid);
    foreach ($query->result() as $row)
    {
        $results[] = array 
        (
    'id' => $row->oldprodsid,
    'src' => 'oldprods',
            'organization' => $row->organization,
            'city' => $row->city,
            'state' => $row->state,
            'country' => $row->country,
            'firstdate' => call_user_func($fmt_date, $row->firstdate),
            'lastdate' => call_user_func($fmt_date, $row->lastdate)
        );
    }

    // IF UPCOMING
    }else if ($view == 'upcoming') {
    $extra_condition = "AND lastdate >= DATE_SUB(CURRENT_DATE,INTERVAL 0 DAY)"; //This line of code designates this as UPCOMING but omit the Legacy Query
    $query = $this->db->query("SELECT distinct customer.customerid as customerid, organization, vcity, vstate, vcountry, firstdate, lastdate 
                               FROM customer, orders, venue 
                               WHERE orders.ordersid=venue.ordersid 
                               AND orders.playid=? AND orders.active=1 
                               AND customer.customerid=orders.customerid 
                               AND shipped != 0 
                               AND defunct != 1 
                               AND orders.lastdate != 0 
                               AND (ordertype != 'misc' 
                               AND ordertype != 'reading') 
                               AND hidepic!=1 
                               AND hideprod!=1 
                               AND num_perf>0 
                               $extra_condition 
                               ORDER BY firstdate desc 
                               LIMIT $offset,$count", $playid);
    foreach ($query->result() as $row)
    {
        $results[] = array 
        (
    'id' => $row->customerid,
    'src' => 'customerid',
            'organization' => $row->organization,
            'city' => $row->vcity,
            'state' => $row->vstate,
            'country' => $row->vcountry,
            'firstdate' => call_user_func($fmt_date, $row->firstdate),
            'lastdate' => call_user_func($fmt_date, $row->lastdate)
        );
    }

    // IF PRO
    }else if ($view == 'pro') {
    $extra_condition = "AND (ordertype = 'pro')"; //This line of code designates this as PRO along with the Legacy Query
    $query = $this->db->query("SELECT distinct customer.customerid as customerid, organization, vcity, vstate, vcountry, firstdate, lastdate 
                               FROM customer, orders, venue 
                               WHERE orders.ordersid=venue.ordersid 
                               AND orders.playid=? AND orders.active=1 
                               AND customer.customerid=orders.customerid 
                               AND shipped != 0 
                               AND defunct != 1 
                               AND orders.lastdate != 0 
                               AND (ordertype != 'misc' 
                               AND ordertype != 'reading') 
                               AND hidepic!=1 
                               AND hideprod!=1 
                               AND num_perf>0 
                               $extra_condition 
                               ORDER BY firstdate desc 
                               LIMIT $offset,$count", $playid);
    foreach ($query->result() as $row)
    {
        $results[] = array 
        (
    'id' => $row->customerid,
    'src' => 'customerid',
            'organization' => $row->organization,
            'city' => $row->vcity,
            'state' => $row->vstate,
            'country' => $row->vcountry,
            'firstdate' => call_user_func($fmt_date, $row->firstdate),
            'lastdate' => call_user_func($fmt_date, $row->lastdate)
        );
    }

    // Legacy data from oldprods
    $query = $this->db->query("SELECT oldprodsid, organization, city, state, country, firstdate, lastdate  
                               FROM oldprods where playid=? 
                               ORDER BY firstdate desc 
                               LIMIT $offset,$count",$playid);
    foreach ($query->result() as $row)
    {
        $results[] = array 
        (
    'id' => $row->oldprodsid,
    'src' => 'oldprods',
            'organization' => $row->organization,
            'city' => $row->city,
            'state' => $row->state,
            'country' => $row->country,
            'firstdate' => call_user_func($fmt_date, $row->firstdate),
            'lastdate' => call_user_func($fmt_date, $row->lastdate)
        );
    }

    }
    return array_slice($results, 0, $count);
}

Answer 1

(What's curious is that you are setting $view = 'all', and then immediately testing if it's set to 'upcoming' or 'pro', I don't see any where else that $view is being assigned a value.)

But, with that issue aside, it took me a bit to figure out that there are huge chunks of code that is replicated. (I actually copied chunks out and saved them as separate files, so I could run compares... and the blocks are IDENTICAL except for the $extra_condition, and the legacy query doesn't get run for 'upcoming', only for 'all' and 'pro'.

I've minimally refactored your code to remove the replicated code. There's probably other changes that can be made, but in terms of a reader figuring out what this code does, removing the redundant code is the first priority.

It would be MUCH easier on the reader if the identical query blocks weren't repeated. In the case of the 'all' view, we can think of $extra_condition as really just an empty string. (The way it's coded below, you'll still get the line break and a bunch of spaces; I might format the query little differently, and include the additions to the WHERE clause a little differently, but I didn't want to change anything that didn't need to be changed.)

Let's break the code into three sections:

• setting $extra_condition
• running the query block
• running the legacy query block

To set $extra_condition, we test whether $view is set to 'all', 'upcoming', or 'pro', and assign the appropriate value. That makes it much easier to see what's going on, when it's set in one place, rather than being spread so far apart in the code.

Then, we'll run the query block. (We don't need to repeat that same code block three times; the code is identical in each case. We just need to check (again) whether $view is set to the one of the three values.

Next, there's a legacy query block that needs to run for two of the $view values. We have this block only once, rather than having two copies of the same code.

The only real difference (and it's not really that different at all) in what gets executed is that the SQL text (query string) for the "all" case is getting an extra empty line. That's because we're including an empty $extra_condition. That extra line is just because of the way that it's getting incorporated into the SQL text. (We could just as easily include a dummy condition like ' AND 1=1 '.)

I wasn't able to post an answer on StackOverflow yesterday because your question got closed before I could post an answer.

<?php
function get_show_($playid)
{

    // There are two tables, so we will need to merge the results for ALL and PRO
    $results = array();

    $count  = ($this->input->get('count')) ? $this->input->get('count') : $this->max_results;
    $offset = ($this->input->get('offset')) ? $this->input->get('offset') : 0;
    $view = 'all';

// SET ADDITIONS TO WHERE CLAUSE FOR QUERY BLOCK

    if ($view == 'all') {
        $extra_condition = ""; //no additions to the WHERE clause
    }else if ($view == 'upcoming') {
        $extra_condition = "AND lastdate >= DATE_SUB(CURRENT_DATE,INTERVAL 0 DAY)"; //This line of code designates this as UPCOMING but omit the Legacy Query
    }else if ($view == 'pro') {
        $extra_condition = "AND (ordertype = 'pro')"; //This line of code designates this as PRO along with the Legacy Query
    }


// RUN QUERY BLOCK

    if (($view == 'all') OR ($view == 'upcoming') OR ($view == 'pro')) {
        $query = $this->db->query("SELECT distinct customer.customerid as customerid, organization, vcity, vstate, vcountry, firstdate, lastdate
                                   FROM customer, orders, venue
                                   WHERE orders.ordersid=venue.ordersid
                                   AND orders.playid=? AND orders.active=1
                                   AND customer.customerid=orders.customerid
                                   AND shipped != 0
                                   AND defunct != 1
                                   AND orders.lastdate != 0
                                   AND (ordertype != 'misc'
                                   AND ordertype != 'reading')
                                   AND hidepic!=1
                                   AND hideprod!=1
                                   AND num_perf>0
                                   $extra_condition
                                   ORDER BY firstdate desc
                                   LIMIT $offset,$count", $playid);
        foreach ($query->result() as $row)
            {
                $results[] = array
                    (
                     'id' => $row->customerid,
                     'src' => 'customerid',
                     'organization' => $row->organization,
                     'city' => $row->vcity,
                     'state' => $row->vstate,
                     'country' => $row->vcountry,
                     'firstdate' => call_user_func($fmt_date, $row->firstdate),
                     'lastdate' => call_user_func($fmt_date, $row->lastdate)
                     );
            }
    }

// RUN LEGACY QUERY BLOCK

    if ( ($view == 'all') OR ($view == 'pro') ) {

        // Legacy data from oldprods
        $query = $this->db->query("SELECT oldprodsid, organization, city, state, country, firstdate, lastdate
                                   FROM oldprods where playid=?
                                   ORDER BY firstdate desc
                                   LIMIT $offset,$count",$playid);
        foreach ($query->result() as $row)
            {
                $results[] = array
                    (
                     'id' => $row->oldprodsid,
                     'src' => 'oldprods',
                     'organization' => $row->organization,
                     'city' => $row->city,
                     'state' => $row->state,
                     'country' => $row->country,
                     'firstdate' => call_user_func($fmt_date, $row->firstdate),
                     'lastdate' => call_user_func($fmt_date, $row->lastdate)
                     );
            }

    }
    return array_slice($results, 0, $count);
}

That's just a lot less code for a reader to wade through. And, if requirements change, and we need to a add a column to the queries, we only have two queries to change, rather than five.

There's a few other minor formatting changes I would make, but for this go around, I changed as few lines as possible. I removed a lot of redundant code, and just relocated and repeated just a few easy conditional tests.