sortByDesc() not Working as intended

Published 2 months ago by SeroS

I have this 2 functions, the problem is that when i use the función sortByDesc, the new collection $clientes is not sorted as it should, if you need more info just ask me, thanks in advance.

public function estado_cc_clientes(){
      $clientes = cliente::select('id','nombre','codigo_empresa','condiciones_pago')->get();

      foreach ($clientes as $cliente) 
      {

        $cliente->cc_final = $this->sacar_cc_final_cliente($cliente->id);
      }

      $clientes = $clientes->sortByDesc('cc_final');

      return view('estadisticas.estado_cc_clientes',compact('clientes'));
    }
public function sacar_cc_final_cliente($cliente_id){
      $cc_debe = cc_clientes::where('cliente_id',$cliente_id)->whereIn('tipo_de_documento',['fc','nd'])->where('borrado',0)->sum('importe');

      $cc_haber = cc_clientes::where('cliente_id',$cliente_id)->whereIn('tipo_de_documento',['rc','nc'])->where('borrado',0)->sum('importe');

      $cc_final = $cc_debe - $cc_haber;

      $cc_final = number_format($cc_final,2);

      return $cc_final;
    }
Best Answer (As Selected By SeroS)
bobbybouwmann

Your code looks correct for sorting. However the problem is in your foreach loop. You loop over each $cliente and assign a value to $client->cc_final. However after the foreach loop the value is never updated so the $clientes is not updated with that value. If you would dd($clientes) after your foreach you would see that the cc_final is not filled.

Eloquent always returns a collection with items. So instead you can use the map method to assign a new value and sort it after that:

$clientes = $clientes->map(function ($cliente) {
    $cliente->cc_final = $this->sacar_cc_final_cliente($cliente->id);
    return $cliente;
})->sortByDesc('cc_final');

That should do the trick

Documentation: https://laravel.com/docs/5.6/collections#method-each

bobbybouwmann

Your code looks correct for sorting. However the problem is in your foreach loop. You loop over each $cliente and assign a value to $client->cc_final. However after the foreach loop the value is never updated so the $clientes is not updated with that value. If you would dd($clientes) after your foreach you would see that the cc_final is not filled.

Eloquent always returns a collection with items. So instead you can use the map method to assign a new value and sort it after that:

$clientes = $clientes->map(function ($cliente) {
    $cliente->cc_final = $this->sacar_cc_final_cliente($cliente->id);
    return $cliente;
})->sortByDesc('cc_final');

That should do the trick

Documentation: https://laravel.com/docs/5.6/collections#method-each

SeroS

Thanks bobbybouwmann, seems like you code is better writed than mine, but the problem was that i made 2 errors.

  1. That the función `sacar_cc_final_cliente was returning a ` string and not a ` float, so that makes the sorting work strange.

  2. I was formating the number on the controller and not in the view, so that makes my code really messy and hard to check for errors.

Thanks for the help.

bobbybouwmann

So did my answer help you in any way? Or did you fix it yourself?

lostdreamer_nl

@bobbybouwmann

$clientes = cliente::select('id','nombre','codigo_empresa','condiciones_pago')->get();
foreach ($clientes as $cliente) 
{
    $cliente->cc_final = $this->sacar_cc_final_cliente($cliente->id);
}

dd($clientes->toArray());

They actually will have cc_final set ;)

As you know, objects are always by reference, so no $clientes as &$cliente) needed.

If it were arrays instead of objects you'd be right ;)

bobbybouwmann

@lostdreamer_nl You're indeed correct! I forgot it was a collection by default.

Anyway, the collection option is cleaner to read in my opinion ;)

SeroS

Sorry for the late reply. The problem was in another part of the code, but your option it's much more cleaner, i'm still starting at this so my code is not as clean as i want it to be.

Thanks for all the help.

Please sign in or create an account to participate in this conversation.