after_get observer not setting values when using get_all()

[awaidus]

HI. Could anybody please tell me how to use after_get observer when getting data through get_all() and return type as "array". after_get observer only works with get() otherwise Undefined index: [table_column_name] error will occur. Please give me at an example with array return_as. Thanks.

[s0x]

This would be resolved by MR #238

[avenirer]

Wouldn't it be better to do a check inside the hook. What I mean by this is the fact that you can check if the array passed to the hook is an associative array (single row) or a numeric array (multiple rows). I would rather not merge the pull request #238, because this won't allow me to see the whole picture of the query. What i mean by this is the fact that the hook won't see if multiple rows were returned and won't know how to act accordingly. What do you say @s0x @salain ?

[s0x]

Well, you do the same for after_create. The observer is called with every id and you dont get the whole collection either. So far I've used after_get for post-processing of the elements and have had no use for dealing with the whole result set for every request. Using the parameter $last still allows you to get the whole picture. If you don't do it this way, what is the purpose of this parameter anyhow?

[avenirer]

That is because on insert, you won't receive anything from the database than eventually the insert id or maybe the total number of rows inserted.

[s0x]

I know... still no difference. You could return a list of IDs there as well, even though I think it is more consistent to have the after_get method to be called with a particular type of value rather than doing additional checks to deal with different types. I never had a situation where I want to do post-processing for ALL result set I retrieve (and hence dealing with that in the model). On the other hand I do post processing for every item a lot. Do you have an example where handling the whole set in after_get would make sense?

[salain]

I agree with @avenirer the after_get hook should get the whole result. I also agree with @s0x, I have yet found what the $last parameter is used for, but I hope to find out eventually. As for a simplified example, I have a case where I need some counter variables that cannot be part of the result and only if I have multiple records. I know I could run some other queries to get these counters but manipulating the array is faster.

[s0x]

I think dealing with this rare case to setup some data based on the collection should be done in the controller anyhow. If there is a good reason to keep it in the model it is quite elegant to realize using the parameter $last:

function after_get($row, $last) {
  // do post processing
  $this->_collected[] = $row
  if ($last) {
    // do your counter setup
   $this->_collected = []
  }
  return $row
}

On the other hand dealing with post processing which requires to distinguish between processing either a row or a set is quite cumbersome. Since I'm pretty sure this is the typical usage scenario and hence used in a lot more models I'd highly recommend to go this way.

function after_get($row) {
  if (!$this->is_assoc($row)) {
    for ($i = 0; i < count($row); $i++) {
      $r = $row[$i]
      // do post processing
      $row[$i] = $r
    }
    // do your counter setup
  } else {
    // do post processing
  }
  return $row
}

In particular this is 4 lines overhead for the less frequent use case while having 7 additional lines and repetition of the post processing for each model dealing with any post processing. I hope you can see why I highly recommend this way! If you still want to keep it this way, I'd recommend to modify the insert post hook and remove the $last parameter from the codebase to have at least some kind of consistency.

[salain]

There is no reason why this should be done in the controller. I do it in the model because it is where the result is the smallest to manipulate as it is before the relations are joined. The usage of the $last parameter is not an issue for me, I just agreed with you. Your first solution would require to duplicate the entire result one row at the time. I don't see the improvement.

[sthiago]

I agree with @s0x on this. I actually made a similar pull request (#146) a few months earlier. Maybe there could be a setting the model definition to choose between the two options. I'm guessing that whichever method one prefers, he/she'll use it consistently throughout the application.