CODEWITHSUNDEEP
phpoptimizationrefactoring
user3848412
user3848412

Reputation: 109

How can I refactor/ simplify the following php function?

I have a function that will return how many products there are on an invoice and it seems pretty straightforward to me but maybe it can use some refactoring?

    public function getTotalProductsNumber(): int
{
    $dataset = 'supplier_invoice_products inner join supplier_invoices as si using (supplier_invoice_id)';
    $dbmSupplier = new Dbm_Supplier($dataset);

    $whereAndOpt = $this->getConditionsAndOptions();
    $where = $whereAndOpt['where'];
    $opt = $whereAndOpt['opt'];
    $select = 'sum(product_quantity) as sumTotal';
    $invoiceTotalProductsNumber =  $dbmSupplier->findFirstSimple($where, $select, $opt);
    $invoiceTotalProductsNumber['sumTotal'] = (int)$invoiceTotalProductsNumber['sumTotal'];
    return $invoiceTotalProductsNumber['sumTotal'];
}

How can I extract this into at least two functions?

Upvotes: 0

Views: 62

Answers (2)

Kevin Y
Kevin Y

Reputation: 711

For this kind of thing, ideally you'd use an ORM. As I'm not sure which you're using, if at all, I'd probably refactor it to something more in this direction so it feels closer to how an ORM would work:

public function getTotalProductsNumber(): int
{
    $dataset = 'supplier_invoice_products inner join supplier_invoices as si using (supplier_invoice_id)';
    $extras = $this->getConditionsAndOptions();
    $queryBuilder = [
        'dbm' => new Dbm_Supplier($dataset),
        'where' =>  $extras['where'],
        'opt' => $extras['opt']
    ];
    return $this->sumOfProductQuantities($queryBuilder);
}

private function sumOfProductQuantities(array $queryBuilder): int
{
    $select = 'sum(product_quantity) as sumTotal';
    $row = $this->queryBuilderFirstRow($queryBuilder, $select);
    return (int)$row['sumTotal'];
}

private function queryBuilderFirstRow(array $qb, string $select): array
{
    return $qb['dbm']->findFirstSimple($qb['where'], $select, $qb['opt']);
}

Upvotes: 1

MLFR2kx
MLFR2kx

Reputation: 1135

I personally think this is pretty straightforward and nicely clean, but the only optimization I can suggest is to extract getConditionsAndOptions part as it seems to be used in some other several parts of your code. Look at this please:

public function getTotalProductsNumber(): int
{
    $dataset = 'supplier_invoice_products inner join supplier_invoices as si using (supplier_invoice_id)';
    $dbmSupplier = new Dbm_Supplier($dataset);

    list($where, $opt) = $this->getWhereAndOpt();

    $select = 'sum(product_quantity) as sumTotal';
    $invoiceTotalProductsNumber = $dbmSupplier->findFirstSimple($where, $select, $opt);
    $invoiceTotalProductsNumber['sumTotal'] = (int)$invoiceTotalProductsNumber['sumTotal'];
    return $invoiceTotalProductsNumber['sumTotal'];
}

private function getWhereAndOpt(): array
{
    $whereAndOpt = $this->getConditionsAndOptions();
    return [
        $whereAndOpt['where'],
        $whereAndOpt['opt'],
    ];
}

Another point maybe name conventions. I can see you have used snail_case naming for Dbm_Supplier. It's better to be either dbm_supplier or DbmSupplier.

Upvotes: 1

Related Questions