Making overloaded functions readable
Sometimes you might allow a function to accept multiple data types. I don't know for certain if it's the correct term but for the remainder of this post I'm going to call such a function overloaded. In this post I'd like to show you a little trick to make overloaded functions more readable.
Let's first take a look at a function in Laravel that accepts multiple data types. To store something in a session you can pass a key and value to the session helper:
session($key, $value);
But you can also give it an array:
session(['key' => 'value']);
Now behind the scenes Laravel is calling a put
function. It could have been implemented like this:
public function put($key, $value = null)
{
if (is_array($key)) {
foreach ($key as $arrayKey => $arrayValue) {
$this->set($arrayKey, $arrayValue);
}
}
else {
$this->set($key, $value);
}
}
In the function above there's a path for doing the work if an array was passed and another path for when (hopefully) a string was passed.
The actual implementation is a bit different (and much better):
public function put($key, $value = null)
{
if (! is_array($key)) {
$key = [$key => $value];
}
foreach ($key as $arrayKey => $arrayValue) {
$this->set($arrayKey, $arrayValue);
}
}
The cool thing to note is that what the function first converts the passed arguments to a certain format (in this case an array) and then perform the work on the format. The actual work, the call to $this->set
is only coded up once. When reading the source code of Laravel you'll often come across this pattern.
Let's take a look at another real life example to make the benefit of this pattern more clear. This next snippet is taken from a recent PR to the laravel-permission package. It aims to a add a query scope to a User
model to perform the query only on users that have the given role(s). Roles can be passed through as an array, a string or an instance of Role
.
/**
* Scope the user query to certain roles only.
*
* @param string|array|Role|\Illuminate\Support\Collection $roles
*
* @return bool
*/
public function scopeRole($query, $roles)
{
if (is_string($roles)) {
return $query->whereHas('roles', function ($query) use ($roles) {
$query->where('name', $roles);
});
}
if ($roles instanceof Role) {
return $query->whereHas('roles', function ($query) use ($roles) {
$query->where('id', $roles->id);
});
}
if (is_array($roles)) {
return $query->whereHas('roles', function ($query) use ($roles) {
$query->where(function ($query) use ($roles) {
foreach ($roles as $role) {
if (is_string($role)) {
$query->orWhere('name', $role);
}
if ($role instanceof Role) {
$query->orWhere('id', $role->id);
}
}
});
});
}
return $query;
}
The query
is being build up in a few different ways depending on the type of the argument being passed through. The readability of this code can vastly be improved by:
- first converting all arguments to a single format to work with
- performing the work on that format
public function scopeRole($query, $roles)
{
if ($roles instanceof Collection) {
$roles = $roles->toArray();
}
if (! is_array($roles)) {
$roles = [$roles];
}
$roles = array_map(function ($role) {
if ($role instanceof Role) {
return $role;
}
return app(Role::class)->findByName($role);
}, $roles);
return $query->whereHas('roles', function ($query) use ($roles) {
$query->where(function ($query) use ($roles) {
foreach ($roles as $role) {
$query->orWhere('id', $role->id);
}
});
});
}
In the snippet above all input, no matter what type is being passed through, is first converted to an array with Role
objects. U sing that array the query is only being build up once. I should mention that the author of the PR did provide a good set of tests so it was very easy to refactor the code.
Let's do one more example. This one's taken from our laravel-fractal package which aims to make working with Fractal more developer friendly.
To return a response with json data you can to this in a Laravel app.
$books = fractal($books, new BookTransformer())->toArray();
return response()->json($books);
In the last version of our package a respond()
method was added. Here's the equivalent code using the respond
method.
return fractal($books, new BookTransformer())->respond();
You can pass a response code as the first parameter and optionally some headers as the second
return fractal($books, new BookTransformer())->respond(403, [
'a-header' => 'a value',
'another-header' => 'another value',
]);
You can also set the status code and the headers using a callback:
use Illuminate\Http\JsonResponse;
return fractal($books, new BookTransformer())->respond(function(JsonResponse $response) {
$response
->setStatusCode(403)
->header('a-header', 'a value')
->withHeaders([
'another-header' => 'another value',
'yet-another-header' => 'yet another value',
]);
});
This is original code for the function that was submitted through a PR (slightly redacted):
public function respond($callbackOrStatusCode = 200, $callbackOrHeaders = [])
{
$response = new JsonResponse();
$response->setData($this->createData()->toArray());
if (is_callable($callbackOrStatusCode)) {
$callbackOrStatusCode($response);
} else {
$response->code($callbackOrStatusCode);
if (is_callable($callbackOrHeaders)) {
$callbackOrHeaders($response);
} else {
$response->withHeaders($callbackOrHeaders);
}
}
return $response;
}
Sure, that code does the job. Unfortunately the real work (in this case: modifying $response
) is done all over the place. Let's refactor! In the code below we're going to convert all input to callable
s first and then use them to modify $response
.
public function respond($statusCode = 200, $headers = [])
{
$response = new JsonResponse();
$response->setData($this->createData()->toArray());
if (is_int($statusCode)) {
$statusCode = function (JsonResponse $response) use ($statusCode) {
return $response->setStatusCode($statusCode);
};
}
if (is_array($headers)) {
$headers = function (JsonResponse $response) use ($headers) {
return $response->withHeaders($headers);
};
}
if (is_callable($statusCode)) {
$statusCode($response);
}
if (is_callable($headers)) {
$headers($response);
}
return $response;
}
Hopefully you can use this neat little trick to improve the readability of your code as well.
What are your thoughts on "Making overloaded functions readable"?