Optimize Codeigniter code

0

I have a system to register debit trading. Suppose you register a negotiation with 3 installments.
Being:

Parcela || Valor  || Vencimento || Situação
1       || 100,00 || 01/09/2017 || Pago
2       || 100,00 || 01/10/2017 || Pago
3       || 100,00 || 01/11/2017 || Aguardando Pagamento

So in the listing, I want to display the status of this deal on:

Acordo em dia
Acordo Quitado
Acordo Quebrado

Following the following rules, if all plots are with pago situation, then the situation will be Acordo Quitado , if there is at least one plot with quebrado situation, then the situation will be Acordo Quebrado and if there is less a parcel with situation Aguardando Pagamento and no Quebrado , then the situation will be Acordo em dia .

I did the following test with the code below and it works, but I believe this is gambiarra,  so I want to know if there is another way of creating the information I want,  using the above rules.

        $this->db->select('situacao');
        $aguardando = $this->db->where('situacao', 0);
        $retorno_aguardando = $this->db->get('tbl_planilha_parcela')->num_rows();

        $quitado = $this->db->where('situacao', 1);
        $retorno_quitado = $this->db->get('tbl_planilha_parcela')->num_rows();

        $quebrado = $this->db->where('situacao', 2);
        $retorno_quebrado = $this->db->get('tbl_planilha_parcela')->num_rows();

        if($retorno_aguardando > 0 && $retorno_quebrado == 0)
        {               
            $situacao = 'Aguardando Pagamento';
        }
        else if($retorno_quebrado > 0)
        {               
            $situacao = 'Acordo Quebrado';
        }
        else if($retorno_aguardando == 0 && $retorno_quitado > 0 && $retorno_quebrado == 0)
        {               
            $situacao = 'Quitado';
        }
    
asked by anonymous 23.10.2017 / 21:22

1 answer

0

If this is within controller then it is not very good.

What I would do:

1 - Create a model to deal with the bank (maybe already do this):

application/models/Acordo_Model.php

defined('BASEPATH') OR exit('No direct script access allowed');

class Acordo_Model extends CI_Model
{
    public function recuperarSituacao($acordo_id) {
        $this->db->query("SELECT situacao, count(situacao) as qtd FROM tbl_planilha_parcela WHERE acordo_id = {$acordo_id} GROUP BY situacao");
        return $this->db->result_array();
    }
}

2º - Your controller should send the agreement id and work the data to know in which situation it is.

application/controllers/situacao :

defined('BASEPATH') OR exit('No direct script access allowed');

class Situacao extends CI_Controller
{
    public function verificar()
    {
        $this->load->model('Acordo_Model', 'acordo');
        $acordo_id = $this->input->get('id');
        $situacoes = $this->acordo->recuperarSituaco($acordo_id);
        $situacao = $this->verificaSituacao($situacoes);

        echo $situacao;
    }

    private function verificaSituacao($items)
    {
        $situacao = array();

        foreach($items as $item) {
            $situacao[$item['situacao']] = $item['qtd'];
        }

        if($situacao[0] > 0 && $situacao[2] == 0) {
            return 'Acordo em dia';
        } else if($situacao[2] > 0) {               
            return  'Acordo Quebrado';
        } else if($situacao[0] == 0 && $situacao[1] > 0 && $situacao[2] == 0) {               
            return 'Acordo Quitado';
        }
    }
}

Finally, I think we have not improved much, I have used exactly their logic of chained ifs, I just optimized the query for a single execution and passing the return to an array and creating a method to do the "check" we avoided having to work with several more variables, the index of the array are exactly the statuses it uses in the database.

I do not believe your approach is a "gambiarra" as I said, I just changed a few things and I introduced acordo_id to make things more readable.

    
27.10.2017 / 20:58