5
\$\begingroup\$

Here is my code:

add_hook('ClientAreaPageInvoices', 1, function($vars) {
    $user_id = $_SESSION["uid"];
    $today = Carbon::now()->format('Y-m-d');
    $query_1 = Capsule::table('tblinvoices')
        ->select(Capsule::raw('SUM(total) as total_aberto'))
        ->where(array('userid' => $user_id, 'status' => 'Unpaid'))
        ->get();
    $query_2 = Capsule::table('tblinvoices')
        ->select(Capsule::raw('SUM(total) as total_vencido'))
        ->where(array('userid' => $user_id, 'status' => 'Unpaid'))
        ->whereDate('duedate', '<=', $today)
        ->get();
    $query_3 = Capsule::table('tblinvoices')
        ->select(Capsule::raw('SUM(total) as total_pago'))
        ->where(array('userid' => $user_id, 'status' => 'Paid'))
        ->get();
    $array_1 = json_decode(json_encode($query_1), True); 
    $array_2 = json_decode(json_encode($query_2), True); 
    $array_3 = json_decode(json_encode($query_3), True); 
    $totalAberto = $array_1[0]['total_aberto'];
    $totalVencido = $array_2[0]['total_vencido'];
    $totalPago = $array_3[0]['total_pago'];
    return array ('totalAberto' => $totalAberto,
                  'totalVencido' => $totalVencido,
                  'totalPago' => $totalPago);
});

Is there any way to simplify the entire code in general?

It works well - I just want to learn other practices to make this type of consultation in a more optimized, and simplified way.

\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

There seems to be a section in your code that you repeat three times. This can probably be placed in a separate function. Something like this:

function getInvoiceTotal($status, $onlyOverdue = FALSE)
{
    $query = Capsule::table('tblinvoices');
    $query->select(Capsule::raw('SUM(total) as total'))
          ->where(['userid' => $_SESSION['uid'], 
                   'status' => $status]);
    if ($onlyOverdue && ($status == 'Unpaid')) {
        $query->whereDate('duedate', '<=', Carbon::now()->format('Y-m-d'));
    }    
    return $query->get()->total;        
}


add_hook('ClientAreaPageInvoices', 1, function($vars) {
    return ['totalAberto'  => getInvoiceTotal('Unpaid'), 
            'totalVencido' => getInvoiceTotal('Unpaid', TRUE), 
            'totalPago'    => getInvoiceTotal('Paid')];
});

Regretably I don't have Laravel installed so I cannot test this code, but I think the general idea is clear. Perhaps someone with more knowledge of Laravel can add to this answer?

Some notes:

  • I changed array(....) to [....].
  • I didn't understand the json_decode(json_encode()) bit. Perhaps to go from objects to arrays? I don't see what's wrong with using the actual objects. Here's where I have most doubt that my code is correct, since I haven't tested it.
  • I left out a lot of variables you used. Using variables, with good names, can make code easier to read, but in really good code, when a variable is only used once, it shouldn't be necessary.
  • I strongly advise you to write your code in English only. I had to find out what the meaning of 'Aberto', 'Vencido' and 'Pago' is. Not every programmer speaks Protuguese.
\$\endgroup\$
4
  • 1
    \$\begingroup\$ When I see json_decode(json_encode()) used (deliberately, correctly), it is to ensure that the whole multi-dimensional iterable structure is all array-type or all object-type. ...but I don't know that this is useful here. \$\endgroup\$ Oct 8, 2020 at 22:22
  • 2
    \$\begingroup\$ @mickmackusa Yes, that's what I assumed, but why is that conversion needed? This has probably to do with my lack of Laravel knowledge. \$\endgroup\$ Oct 8, 2020 at 22:22
  • 1
    \$\begingroup\$ @KIKOSoftware Very cool your code, really with a separate function everything is simplified, however it did not work for me, it returned all zero (0,00) values. \$\endgroup\$
    – zecaluis
    Oct 9, 2020 at 1:09
  • \$\begingroup\$ It needs some basic debugging. The idea is sound, but clearly there's still some kind of error in it. \$\endgroup\$ Oct 9, 2020 at 7:47
0
\$\begingroup\$

SUM aggregates

Instead of selecting the SUM aggregate manually:

->select(Capsule::raw('SUM(total) as total_aberto'))

The Builder::sum() aggregate method can be used:

->sum('total')

Is there a model defined like Invoice?

<?php

namespace App\Models;

class Invoice extends Model 
{

    protected $table = 'tblinvoices';

}

Presuming such a model existed, the code like this:

$query_1 = Capsule::table('tblinvoices')
    ->select(Capsule::raw('SUM(total) as total_aberto'))
    ->where(array('userid' => $user_id, 'status' => 'Unpaid'))
    ->get();

could be simplified to this, using the methods to retrieve Single Models / Aggregates:

$totalAberto = App\Models\Invoice::where(['userid' => $user_id, 'status' => 'Unpaid'])
                                  ->sum('total');
    

Array syntax

There isn't anything wrong with using array() but as of the time of writing, PHP has Active support for versions 8.0 and 7.4, and since PHP 5.4 arrays can be declared with short array syntax (PHP 5.4), as was used in my sample at the end of the section above.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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