Repeated code in several functions with PHP

The answer may be to move the "unique code" to the different object class, if it depends only on the object type.

If it depends on factors other than the object type, then indeed a good approach is a "callback"

private function commonFunction($arg1, $arg2, $differentObject, $uniqueCallback) { // $arg1 and $arg2 are common to all functions if ($firstCheck) { // some code if($secondCheck) { // some code if ($thirdCheck) { $uniqueCallback($differentObject); } } } // return statement } public function similarFunction($arg1, $arg2, $differentObject) { $this->commonFunction($arg1, $arg2, $differentObject, function($differentObject) { // uniqueCode }); } 

It is hard to advise you without knowing exactly what your data represents.

If the args are the same for all function and you do some operations on them before the uniqueCode operations wouldn't it be better to create an object where you would have your args as parameters and your checks as methods?

In other words the uniquecode seems to be the core of your function. Leave it visible and readable and refactor the validation part.

Something like:

class ValidateArgs { private $arg1; private $arg2; public function __construct($arg1, $arg2) { $this->arg1 = $arg1; $this->arg2 = $arg2; } public function check() { //Check $check = $this->arg1 && $this->arg2; return $check; } } 

And then you would call it like:

public function similarName($arg1, $arg2, $differentObject) { $validation = new ValidateArgs($arg1, $arg2); if($validation->check()) { // unique code goes here return $result_of_unique_code; } } 

UPDATE:

After seeing your code example I believe you need multiple iterations to successfully refactor it. Go slowly one step at a time trying to get the code a little bit cleaner on each step.

Here are a couple of suggestions:

Simplify the if/else structure. Sometimes you can avoid the use of the else part entirely. For instance you can check for $request->getMethod() and return immediately:

if ($request->getMethod() != 'POST') { return false; } //Rest of the code 

The count($errorList) validation seems to depend only on data['file']. I guess you could create a function with all that logic. Something like this:

public function constrainValidations($data) { $imageConstraint = new \Symfony\Component\Validator\Constraints\Image(); $imageConstraint->maxSizeMessage = Image::ERROR_MESSAGE; $imageConstraint->maxSize = Image::MAX_SIZE; $errorList = $this->validator->validateValue($data['file'], $imageConstraint); if (count($errorList) == 0) { return true; } else { return false; } } 

Then your original code would start to look a little bit more clean and readable:

if ($request->getMethod() != 'POST') { return false; } if (!$this->constrainValidations($data)) { return false; } //Unique code goes here return true; 

Keep doing it step by step. Trying to unnest all the if statements. Also this will makes it possible for you to change the return statements and start throwing Exceptions.

Then you can possibly start to think on the object approach for extra readability.

I would personally avoid any solution that involves a callback, but thats a matter of taste.