How to Refactor a Method With Optional Params
Tags: refactoring, programming, php, webdev
Published: 2022-02-26 07:44:08

While refactoring and optimising legacy code I've ran into a method signature (optional parameters) inconsistency which caused performance issues as part of the expensive operation was being done without developer intention.

```php
class PageRepository
{
    ...
    public function find($pageId, $prerender = false)
    {
        ...
        if ($prerender === true) {
            $page->prerender();
        }
        ...
	return $page;
    }

    public function findOrFail($pageId, $prerender = true)
    {
        $page = $this->find($pageId, $prerender);

        if ($page === null) {
            throw new Exception("Page $pageId not found.");
        }
				
	return $page
    }
    ...
}
```
Our `PageRepository::find` method had an optional parameter called `$prerender` which was set to `false` by default. The problem is that the method `PageRepository::findOrFail` has the same optional parameter but its default value set to `true`! 

If you're not careful reading method signature and default param values you can easily make a mistake and "prerender" a page (which is an expensive operation) without wanting or needing it.

Methods with optional "flag" parameters can potentially be dangerous and can be hard to glance at.

There are a couple of things that can be done here in order for it to be better and more readable.

## 1. Change optional parameter default values to the same value

This one is a no brainer. Just change the `PageRepository::findOrFail` optional `$prerender` default value to `false` (or vice-versa). 

## 2. Specify optional param value when calling methods that have them

Just specify a value explicitly so anyone reading your code (even you a couple of months in the future) will know that you've set it and used it intentionally.

```php
$pageRepository = new PageRepository();

$page = $pageRepository->find($pageId, false);
```

Or even better, use the variable with a semantic meaning when calling the method.

```php
$pageRepository = new PageRepository();

$page = $pageRepository->find($pageId, $prerender = false);
```
_I know that some linters or IDEs may highlight the `$prerender` param but there are several ways to deal with it and I'm leaving it at your discretion to chose a way how to handle it._

## 3. Refactor the method with optional "flag" parameter into two distinct methods

Remove the `$page->prerender()` call from the `PageRepository::find` method and extract it to a separate `PageRepository::findAndPrerender` method.

```php
class PageRepository
{
    ...
    public function find($pageId, $prerender = false)
    {
        ...
        if ($prerender === true) {
            $page->prerender();
        }
        ...
        
        return $page;
    }

    public function findAndPrerender($pageId)
    {
        $page = $this->find($pageId, $prerender);

        if ($page === null) {
            throw new Exception("Page $pageId not found.");
        }

        return $page;
    }
    ...
}
```
This way you have to make an intentional effort to chose which functionality (or rather which side-effect) you require.

This was just a quick refactoring tip which I’m usually enforcing suggesting while doing code reviews in order to make code easily understandable and to avoid confusion.