[5.5] Refactor resolve method to make more readable

[georgehanson]

In the pull request I have altered the resolve method within the Resource class. It seems a bit odd in my opinion to be setting a variable to itself $data = $data. Therefore I have altered this slightly to remove it and make it more readable (in my opinion)

[JosephSilber]

A Collection is Arrayable.

[georgehanson]

Fair point

[JosephSilber]

Why are you casting it to an array again?

[georgehanson]

Good spot. I only re-arranged the code, I didn't look too much into what it was doing. I will amend this

[ConnorVG]

I feel like:

if ($data instanceof Arrayable) {
    $data = $data->toArray();
} elseif ($data instanceof JsonSerializable) {
    $data = $data->jsonSerialize();
}

return $this->filter((array) $data);

Better fits Laravel's code style.

[georgehanson]

I've updated as per @ConnorVG suggestion. I've kept the (array) cast out however, because jsonSerialize() and toArray() both return arrays

[JosephSilber]

Why are you calling filter here & above?

[georgehanson]

Oops...

[ConnorVG]

You need to not call filter so many times (no early return or pre-filtering) and you need to account for arrayable instances that don't implement Arrayable or JsonSerializable, such as stdClass.

stdClass is neither Arrayable or JsonSerializable but can be cast to an array (array) $obj and can be serialized json_encode($obj).

Keep the (array).

[georgehanson]

Changes have been made

[ConnorVG]

Cool. Now this PR simply just removes excess code without losing functionality or changing the results.

😄

[georgehanson]

Yup, got there in the end @ConnorVG 😆

[sisve]

Wouldn't it be appropriate to add some tests to ensure that the scenarios discussed here is supported in the future to? Anything that verifies that it still works with arrays, collections and stdObjects.

[taylorotwell]

Unless something is actually broken, I see no reason to change this. If there is a bug feel free to open another PR with a failing test and this change.