PHP 的几个函数中的重复代码

Repeated code in several functions with PHP

我正在向 php class 添加一些代码,其中有几个函数包含我想要重构的重复代码。这些函数如下所示:

public function similarName($arg1, $arg2, $differentObject) 
{
    // $arg1 and $arg2 are common to all functions
    if ($firstCheck) {
        // some code
            if($secondCheck) {
                // some code
                if ($thirdCheck) {
                    // unique code with $differentObject
                }
            }
        }
    // return statement
}

我现在需要添加一个新的函数,下面完全一样,但是代码是唯一的。

以下是我想到的事情:

正确的模式是什么?

更新:

下面是其中两个函数的真实代码。它们是为对象添加上传图像的 Symfony2 表单处理程序

public function handleToPost(FormInterface $form, Request $request, Post $post)
{
    if ($request->getMethod() == 'POST') {
        $form->bind($request);

        $data = $form->getData();
        $file = $data['file'];
        if($data['file']) {
            $imageConstraint = new \Symfony\Component\Validator\Constraints\Image();
            $imageConstraint->maxSizeMessage = Image::ERROR_MESSAGE;
            $imageConstraint->maxSize = Image::MAX_SIZE;
            $errorList = $this->validator->validateValue($file, $imageConstraint);
            if (count($errorList) == 0) {
                if (!$post->getImage()) {
                    $image = new ImagePost();
                    $image->setPost($post);
                    $image->setFile($file);
                    $this->imageManager->saveImage($image);
                } else {
                    $image = $post->getImage();
                    $image->setFile($file);
                }
                $this->imageManager->createImage($image);
            } else {
                return false;
            }

            return true;
        }
    }

    return false;
}

public function handleToEvent(FormInterface $form, Request $request, Event $event)
{
    if ($request->getMethod() == 'POST') {
        $form->bind($request);
        $data = $form->getData();
        $file = $data['file'];
        if ($data['file']) {
            $imageConstraint = new \Symfony\Component\Validator\Constraints\Image();
            $imageConstraint->maxSizeMessage = Image::ERROR_MESSAGE;
            $imageConstraint->maxSize = Image::MAX_SIZE;
            $errorList = $this->validator->validateValue($file, $imageConstraint);
            if (count($errorList) == 0) {
                if (!$event->getImage()) {
                    $image = new ImageEvent();
                    $image->setEvent($event);
                    $image->setFile($file);
                    $this->imageManager->saveImage($image);
                } else {
                    $image = $event->getImage();
                    $image->setFile($file);
                }
                $this->imageManager->createImage($image);
            } else {
                return false;
            }
            return true;
        } else {
            return true;
        }
    }

    return false;
}

答案可能是将"unique code"移动到不同的对象class,如果它仅取决于对象类型。

如果它取决于对象类型以外的因素,那么确实一个好的方法是 "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
        });
}

如果不确切知道您的数据代表什么,很难给您建议。

如果所有函数的 args 都相同,并且您在 uniqueCode 操作之前对它们进行了一些操作,那么创建一个将 args 作为参数和检查的对象不是更好吗作为方法?

换句话说,uniquecode 似乎是您函数的核心。保持可见和可读并重构验证部分。

类似于:

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;
    }
}

然后你会这样称呼它:

public function similarName($arg1, $arg2, $differentObject) 
{
    $validation = new ValidateArgs($arg1, $arg2);
    if($validation->check()) {

        // unique code goes here

        return $result_of_unique_code;
    }
}

更新:

看到您的代码示例后,我相信您需要多次迭代才能成功重构它。一步一步慢慢来,尝试让代码在每一步都更清晰一点。

这里有一些建议:

简化if/else结构。有时您可以完全避免使用 else 部分。例如,您可以立即检查 $request->getMethod() 和 return:

if ($request->getMethod() != 'POST') {
    return false;
}

//Rest of the code

count($errorList) 验证似乎只依赖于 data['file']。我猜你可以创建一个包含所有这些逻辑的函数。像这样:

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;
    }
}

那么您的原始代码将开始看起来更简洁和可读:

if ($request->getMethod() != 'POST') {
    return false;
}

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

继续一步一步来。试图解除所有 if 语句的嵌套。 此外,这将使您可以更改 return 语句并开始抛出异常。

然后您可以开始考虑对象方法以获得额外的可读性。

我个人会避免任何涉及回调的解决方案,但那是个人喜好问题。

问得好。

基于这两个示例,Post 和事件 class 似乎都应该实现某种 "ImageSource" 接口。如果其他情况类似,并且还假设事件、Post 和其他 class 可以轻松更改,在我看来代码应该是这样的。让我们调用公共函数 "handleImageSource":

public function handleImageSource(FormInterface $form, Request $request, ImageSource $imgsrc)
{
    if ($request->getMethod() == 'POST') {
        $form->bind($request);
        $data = $form->getData();
        $file = $data['file'];
        if ($data['file']) {
            $imageConstraint = new \Symfony\Component\Validator\Constraints\Image();
            $imageConstraint->maxSizeMessage = Image::ERROR_MESSAGE;
            $imageConstraint->maxSize = Image::MAX_SIZE;
            $errorList = $this->validator->validateValue($file, $imageConstraint);
            if (count($errorList) == 0) {
                $image = $imgsrc->createImageFromFile($file, $this->imageManager);
            } else {
                return false;
            }
            return true;
        } else {
            return true;
        }
    }

    return false;
}

那么,每个实现ImageSource接口的class都应该有这样一个方法。例如在事件 class:

public function createImageFromFile($file, $imageManager)
{
    if (!($image = $this->getImage()) ) {
        $image = new ImageEvent();//|| new ImagePost() || etc...
        $image->setEvent($this);//|| setPost() || etc...
        $image->setFile($file);
        $imageManager->saveImage($image);
    } else {
        $image->setFile($file);
    }
    $imageManager->createImage($image);
    return $image;
}

在其他情况下,或者如果您只想用 "handleToXxxx" 方法重构 class,我会在每次调用之前用不同的代码创建一个匿名函数。例如,再次使用事件 class:

$image_source = function($file, $imageManager) use ($Event){
    if (!($image = $Event->getImage()) ) {
        $image = new ImageEvent();//|| new ImagePost() || etc...
        $image->setEvent($this);//|| setPost() || etc...
        $image->setFile($file);
        $imageManager->saveImage($image);
    } else {
        $image->setFile($file);
    }
    $imageManager->createImage($image);
    return $image;
};
//Then call to the function
$theHandleObj->handleImageSource($form, $request, $image_source);

然后,在“$theHandleObj”中class:

public function handleImageSource(FormInterface $form, Request $request, callable $imgsrc)
{
    if ($request->getMethod() == 'POST') {
        $form->bind($request);
        $data = $form->getData();
        $file = $data['file'];
        if ($data['file']) {
            $imageConstraint = new \Symfony\Component\Validator\Constraints\Image();
            $imageConstraint->maxSizeMessage = Image::ERROR_MESSAGE;
            $imageConstraint->maxSize = Image::MAX_SIZE;
            $errorList = $this->validator->validateValue($file, $imageConstraint);
            if (count($errorList) == 0) {
                $image = $imgsrc($file, $this->imageManager);
            } else {
                return false;
            }
            return true;
        } else {
            return true;
        }
    }

    return false;
}

希望这对您有所帮助:)