Joining multiple functions data in one array

Question

This bit of code gathers data from two methods and puts it into an array. The code works fine but it doesn't look pretty and I feel like there is a better way of doing what I have done.

public static function RagUpdate()
{
 $MachineOnOffStatus = self::RagOnOffStatus();
 $ActiveMachine = array();
 foreach ($MachineOnOffStatus as $Key => $val) {
 if ($val['DateDifference'] < 360) {
 $ActiveMachine[$val['controllerID']]['controllerID'] = $val['controllerID'];
 $ActiveMachine[$val['controllerID']]["DateDiff"] = $val['DateDifference'];
 }
 }
 foreach (self::ActiveMachineCycleTime($ActiveMachine) as $Key => $val) {
 foreach ($ActiveMachine as $k => $v) {
 if ($val["controllerID"] == $v["controllerID"]) {
 $ActiveMachine[$v['controllerID']]["CycleTime"] = $val['ElaspedTime'];
 }
 }
 }
 return $ActiveMachine;
}

Output of this method:

array (size=5)
2 => 
array (size=3)
 'controllerID' => string '2' (length=1)
 'DateDiff' => string '136' (length=3)
 'CycleTime' => string '-61' (length=3)
28 => 
array (size=3)
 'controllerID' => string '28' (length=2)
 'DateDiff' => string '129' (length=3)
 'CycleTime' => string '-52' (length=3)
30 => 
array (size=3)
 'controllerID' => string '30' (length=2)
 'DateDiff' => string '96' (length=2)
 'CycleTime' => string '-45' (length=3)
37 => 
array (size=3)
 'controllerID' => string '37' (length=2)
 'DateDiff' => string '123' (length=3)
 'CycleTime' => string '-69' (length=3)
40 => 
array (size=3)
 'controllerID' => string '40' (length=2)
 'DateDiff' => string '89' (length=2)
 'CycleTime' => string '-75' (length=3)

Answer 1

I see some redundancy right off the bat:

$ActiveMachine[$val['controllerID']]['controllerID'] = $val['controllerID'];

If the key is the controllerID, do you need a separate controllerID underneath of it? I would suspect not. You can use the $Key in place of $val['controllerID'] in the foreach below it.

As for the rest, do you have control over the output of the functions you are using? Can the key of those arrays be used in a smarter way? You are using $Key => $val, but you don't use $Key once.

Answer 2

I think you should be doing something like the following when processing the MachineOnOffStatus data:

$ActiveMachine[$val['controllerID']] = $val;

This allows you to find and access information by controllerID. Having done something like that you can probably avoid comparing every active machine to ever active machine cycle time to look for a matching controllerID.

However, perhaps $val contains a lot of spurious data you don't need?

Anyway, using the ID (or what should be the key) in this way would allow you to perform a key based lookup on the other as long as each was stored by key.

I'm also curious why you are working with lists. The language used would naturally refer to getting the status of a single machine:

$MachineOnOffStatus = self::RagOnOffStatus();

If you had a set of machines you could loop through them querying each for on/off status and cycle time data storing the result by machineID as you do so.

Perhaps you could provide a little more background material concerning why you have structured the code the way you have?

Also, in terms of simple code review, you are asking for trouble using very similarly named variables.