1
\$\begingroup\$

I have this piece of code that is used to export an Excel file using Laravel-Excel. But I have doubts about it for the long run. Can the server handle it if there are a hundred requests?

<?php

namespace App\Exports;

use Maatwebsite\Excel\Concerns\FromCollection;
use Maatwebsite\Excel\Concerns\Exportable;
use Maatwebsite\Excel\Concerns\WithHeadings;
use Carbon\Carbon;
use Carbon\CarbonPeriod;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Arr;

class DailyGeneralSheetExport implements FromCollection, WithHeadings
{
    use Exportable;

    public function __construct(array $request, int $diid, int $igid, str $date)
    {
        $this->request = $request;
        $this->diid = $diid;
        $this->igid = $igid;
        $this->date = $date;
    }

    public function collection()
    {
        $dt = Carbon::parse($this->date);
        $endDt = Carbon::parse($this->date)->lastOfMonth();
        $period = CarbonPeriod::create($this->date, $endDt);
        $ipid = array_pluck($this->request, 'ip_id');
        $inidArr = array_pluck($this->request, 'in_id');
        $inid = array_unique($inidArr);
        $collection = [];
        foreach ($period as $key => $eachDate) {
            $reading = DB::table('instrument')
                    ->where('iv_inid', $inid)
                    ->whereIn('iv_ipid', $ipid)
                    ->whereDate('iv_date', $eachDate)
                    ->whereIn('iv_status', ['N', 'Y', 'A'])
                    ->orderBy('iv_date')->get()->toArray();
            $row = array($eachDate->format('d/m/Y'));
            foreach ($reading as $columnKey => $columnValue) {
                array_push($row, $columnValue->iv_reading);
            }
            array_push($collection, $row);
        }

        return collect($collection);
    }

    /**
     * @return array
     */
    public function headings(): array
    {
        $sheet_header = Arr::pluck($this->request, 'ip_label');

        return Arr::prepend($sheet_header, 'Date');
    }
}

As you can see I have to create an array for one month and this makes me worry if this will impact the performance. And for some reason, I feel I wrote code that is not elegant as Laravel does. How do I optimize it?

\$\endgroup\$
  • \$\begingroup\$ Have you tried load testing to see what happens with N hits at once? \$\endgroup\$ – pacmaninbw Jan 1 at 16:07
1
\$\begingroup\$

The code looks OK. You may want to move the query outside of the foreach loop and group the results afterwards. But the foreach loop is limited to the number of days in a month, so I don't think that will do very much. That may cause the application to use more memory.

Therefore, I advise testing if the performance is indeed an issue. If so you might want to delegate creating the export to a queue. Queues in Laravel are not hard to implement (surprise!)

Laravel Queues Documentation 7.x

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Welcome to CodeReview@SE. This looks an assessment of a man-machine interface issue. I am not sure this provides me with an insight about the code presented. \$\endgroup\$ – greybeard Mar 17 at 3:57
  • \$\begingroup\$ True, but I did not see any chance for optimising the code - it looks great. \$\endgroup\$ – Erlend ter Maat Mar 17 at 15:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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