Better refactoring of map function

Question

I'm currently refactoring my methods. I already eliminated long foreach and if states but now i'm struggling a bit with last finishing to have a nice map function.

My method looks like.

/**
 * @param \Illuminate\Support\Collection
 * @return array
 */
public static function convertTimesToChartData($times) {
    $clientTotal = '0';
    $arrayIndex = 0;
    $chartData = collect($times)->filter(function($item) {
        return !is_null($item->end_time);
    })->map(function($item) use(&$clientTotal, &$arrayIndex){
        $clients[$arrayIndex]['id'] = $item->client_id;
        $clients[$arrayIndex]['project_id'] = $item->project_id;
        $clients[$arrayIndex]['label'] = $item->company;
        $clients[$arrayIndex]['sec'] = $item->end_time->timestamp - $item->start_time->timestamp;
        $clientTotal += $item->end_time->timestamp - $item->start_time->timestamp;
        $arrayIndex++;
        return $clients;
    })->flatMap(function($item) { return $item; });


    return $chartData;
}

Here, i have 2 questions:

Is there a better way assign the array? When i'm trying to assigning directly in the return like

        return [
        [$arrayIndex]['id'] => $item->client_id,
        [$arrayIndex]['project_id'] => $item->project_id,
        [$arrayIndex]['label'] => $item->company,
        [$arrayIndex]['sec'] => $item->end_time->timestamp - $item->start_time->timestamp,
    ];

But then i get an undefined index error.

How is the best way to return a summarized array? cause i have several time entries for same client, so at the end, i just want the summarized seconds. It works on my example but i think there are better ways to do that. Specially because i need to define an $arrayIndex which i need to declare outside of the map function and reference to it. Don't think that this is the best way.

Here is the original source:

    $projects = array();
    $projectTotal = '0';
    foreach($taskTimes as $time){
        if(!is_null($time->end_time))
        {
            if (isset($projects[$time->project_id]))
            {
                $projects[$time->project_id]['sec'] += $time->end_time->timestamp - $time->start_time->timestamp;
            } else
            {
                $projects[$time->project_id]['id'] = $time->client_id;
                $projects[$time->project_id]['label'] = $time->company;
                $projects[$time->project_id]['sec'] = $time->end_time->timestamp - $time->start_time->timestamp;
            }
            $projectTotal += $time->end_time->timestamp - $time->start_time->timestamp;
        }
    }

Thanks for any advice and help!

Do you still have a copy of the original "long foreach and if states"? — Rwd, Mar 12 '17 at 12:12

score 1 · Accepted Answer · answered Mar 12 '17 at 15:25

To get the same results you were getting from your original foreach loop you could so something like:

$projects = collect($taskTimes)
    ->filter(function ($time) {
        return !is_null($time->end_time);
    })
    ->groupBy('project_id')
    ->map(function ($group) {
        $group = collect($group);

        return [
            'id'         => $group->first()->client_id,
            'project_id' => $group->first()->project_id,
            'label'      => $group->first()->company,
            'sec'        => $group->sum(function ($item) {
                return $item->end_time->timestamp - $item->start_time->timestamp;
            }),
        ];
    });

$projectsTotal = $projects->sum('sec');

I've added the project_id to the resulting array as it was included in your convertTimesToChartData() method example

Hope this helps!

Better refactoring of map function

1 Answers1