Refactoring a PHP class
I am playing around with refactoring a PHP class. My original class had a constructor and one big method that manipulated the string through various operations and spit out the result at the end.
class String
{
public function __contstruct()
{
// a couple of initialisation things
}
public function make($string)
{
// very large method that does lots of different types of filtering to an input string
}
}
My class is run through scrutinizer-ci and one of the things it suggested was to refactor the large method into smaller standalone methods using the Composer method pattern.
https://scrutinizer-ci.com/docs/refactorings/compose-method
I did this and it looks neater, but here is my problem / request ...
My new class now looks something like this:
class String
{
public function __construct()
{
// a couple of initialisation things
}
public function make($string)
{
$string = $this->smallMethodOne($string);
$string = $this->smallMethodTwo($string);
$string = $this->smallMethodThree($string);
$string = $this->smallMethodFour($string);
$string = $this->smallMethodFive($string);
}
private function smallMethodOne($string) {
// do some stuff
return $string;
}
private function smallMethodTwo($string) {
// do some stuff
return $string;
}
private function smallMethodTwo($string) {
// do some stuff
return $string;
}
private function smallMethodThree($string) {
// do some stuff
return $string;
}
private function smallMethodFour($string) {
// do some stuff
return $string;
}
private function smallMethodFive($string) {
// do some stuff
return $string;
}
}
I'm just wondering if there was a better way to organize the method make()
, as it doesn't feel right doing it like this, just executing a sequence of methods in sequence.
source to share
What you should be asking yourself is, how difficult is it to override one "smallMethod" without changing the behavior of the other "smallMethod"?
If you can override one "smallMethod" without touching the others, you are probably doing the right thing.
Say you have this:
public function deleteMe()
{
if($this->a == x || $this->b != y){
$this->cache->delete($this);
$this->storage->delete($this);
$this->refs->unlink($this);
}
}
Now if you want to extend the class and change the deleteMe()
if () condition ? You will need to completely rewrite the method to change one line. It's not convenient at all. Instead of this:
protected function checkDeletability()
{
return $this->a == x || $this->b != y;
}
protected function internalDelete()
{
$this->cache->delete($this);
$this->storage->delete($this);
$this->refs->unlink($this);
}
public function deleteMe()
{
if($this->checkDeletability())
$this->internalDelete();
}
This way, you can override parts of the procedure without touching others. That is why you "decomposed the big method into smaller ones."
So, check if you are satisfied with this. It is most important!
source to share
I would like to:
public function __construct($string)
{
$this->string = $string;
// a couple of initialisation things
}
public function make()
{
if(!$this->smallMethodOne()){
return false;//or custom exception or array with status/description
}
...
if(!$this->smallMethodFive()){
return false;//or custom exception or array with status/description
}
return $this->string;
}
private function smallMethodOne() {
// do some stuff
$this->string = $new_string;
}
source to share