Optimise advice for many loops in php?

Question

I have been working on this code piecing it together here and there as it came and works, and the result is VERY MESSY!

I just need some advice what i should do to reduce the amount of loops or perhaps are there any loops you see that shouldn't be needed?

any advice to the below code is appreciated.

if (isset($_POST['refresh-history'])):
  $order_id = $_POST['id'];
  $order = $database->get_results('SELECT * FROM `orders` WHERE `order_id`='.$order_id);
  $matches = $database->get_results('SELECT `match_id` FROM `matches` WHERE `order_id`='.$order_id);
  foreach ($order as $o):
    $loluser = $o->loluser;
    $region = $o->region;
    $date_created = $o->date_created;
    $date_completed = $o->date_completed;
  endforeach;
  $api->setRegion($region);
  $matchlistapi = $api->matchlist();
  $matchapi = $api->match();
  $matchlist = $matchlistapi->matchlist($loluser, "RANKED_SOLO_5x5", "SEASON2015", null, null, null, $date_created, $date_completed);
  if ($matchlist->totalGames !== 0):
    foreach ($matchlist as $key):
      $gameIds[] = $key->matchId;
    endforeach;
    $arr_matches = object2array($matches);
    foreach ($arr_matches as $id) {
      $dbMatches[] = (int)$id->match_id;
    }
    $new_array = array_diff($gameIds, $dbMatches);

    foreach ($new_array as $matchId):
      $games[] = $matchapi->match($matchId, false);
    endforeach;
    foreach ($games as $game):
      // Store Games in DB;
    endforeach;
    $_SESSION['api_success'] = "Success: Games Synced to Order.";
  else:
    $_SESSION['error_msg'] = "Error 23: Unable to Find Games.";
  endif;
endif;

To be Clear I DO NOT NEED AN ANSWER! Just a PUSH in the right direction, i can go from there. :)

Answer 1

Explanation

You can replace the following code

foreach ($order as $o):
    $loluser = $o->loluser;
    $region = $o->region;
    $date_created = $o->date_created;
    $date_completed = $o->date_completed;
endforeach;

with

//get the last order.
$order = end($orders);

//set the information.
$loluser = ($order === false) ? '' : $order->loluser;
$region = ($order === false) ? '' : $order->region;
$date_created = ($order === false) ? '' : $order->date_created;
$date_completed = ($order === false) ? '' : $order->date_completed;

There are solved two problems. The first thing is, that your foreach run through all orders and write the properties of every order to the variables. At the end only the properties of the last order will be set on variables. The second thing is, that if no order is available the variables are not set on the following script.

On the sync part i could remove some variables which are not needed because they only stores an array for the next loop. But you can use the result of the function itself on the loop if you don't check the result of the functions before.

An example. You can replace the following code

$new_array = array_diff($gameIds, $dbMatches);

foreach ($new_array as $matchId):
    $games[] = $matchapi->match($matchId, false);
endforeach;

can be replaced with

foreach (array_diff($gameIds, $dbMatches) as $matchId):
    $games[] = $matchapi->match($matchId, false);
endforeach;

The Result

Here you can find the full code of your script with some optimizations:

<?php
if (isset($_POST['refresh-history'])) {
    $order_id = $_POST['id'];
    $orders = $database->get_results('SELECT * FROM `orders` WHERE `order_id` = '.$order_id);
    $matches = $database->get_results('SELECT `match_id` FROM `matches` WHERE `order_id` = '.$order_id);

    //get the last order.
    $order = end($orders);

    //set the information.
    $loluser = ($order === false) ? '' : $order->loluser;
    $region = ($order === false) ? '' : $order->region;
    $date_created = ($order === false) ? '' : $order->date_created;
    $date_completed = ($order === false) ? '' : $order->date_completed;

    $api->setRegion($region);
    $matchlistapi = $api->matchlist();
    $matchapi = $api->match();
    $matchlist = $matchlistapi->matchlist($loluser, "RANKED_SOLO_5x5", "SEASON2015", null, null, null, $date_created, $date_completed);

    //check if a game is available.
    if ($matchlist->totalGames > 0) {

        //initialize the vars.
        $matchIds = array();
        $matchIdsDB = array();

        //collect all match ids from api.
        foreach ($matchlist as $match) {
            $matchIds[] = (int) $match->matchId;
        }

        //collect all match ids from database.
        foreach (object2array($matches) as $match) {
            $matchIdsDB[] = (int) $match->match_id;
        }

        //run through all missing matches.
        foreach (array_diff($matchIds, $matchIdsDB) as $match) {
             $game = $matchapi->match($match, false);

             //store game in database or create a big query to create all in one.
        }

        $_SESSION['api_success'] = "Success: Games Synced to Order.";
    } else {
        $_SESSION['error_msg'] = "Error 23: Unable to Find Games.";
    }
}

Hope it helps!

Answer 2

This code is doing the following:

• Get the information in the DB corresponding to the current order:

  'SELECT * FROM `orders` WHERE `order_id` = '.$order_id);
  

Here the problem is that order_id should be your primary key so no loop is needed.

• Get the match lists matching the current order with the API

    $matchlistapi->matchlist(...)
  

• Get the match lists matching the current order within the Database

• Store games in the DB which are not already present

All that is useless. What is important is to store the match in a unique way.

Simply define a multi column unique key on your matches tables

ALTER TABLE matches ADD UNIQUE (order_id, ..., ...)

and use INSERT IGNORE INTO matches when you will insert.

So the elements which already are in the matches table won't be overwritten.

Your code will be something similar to:

$result = $database->get_results('SELECT loluser, region, date_created, date_completed
             FROM `orders` WHERE `order_id` = '.$order_id);

list($loluser, $region, $date_created, $date_completed)=$result[0];

$matchlistapi = $api->matchlist();
$matchlist = $matchlistapi->matchlist($loluser, "RANKED_SOLO_5x5",
            "SEASON2015", null, null, null, $date_created, $date_completed);

$matchapi = $api->match();
foreach ($matchlist as $m) {
   $match=$matchapi->match($m->match_id, false);
   $stmt=$database->prepare("INSERT IGNORE INTO matches VALUES(?, ?, ?)", ...);
   $database->execute($stmt);
}

Note: Of course you have to do care about potential SQL injection in your first request