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
}
我现在需要添加一个新的函数,下面完全一样,但是代码是唯一的。
以下是我想到的事情:
- 使用回调函数作为参数,不知道怎么做。
- 使用普通代码的单个函数,return一个布尔值。如果为真,则在每个函数中执行唯一代码。这对我来说似乎不够优雅。
- 对唯一代码使用抽象class和模板抽象方法,但我不确定如何传递 $differentObject。
- 我什至怀疑这是不是装饰器模式的问题,但我从来没有这样使用过。
正确的模式是什么?
更新:
下面是其中两个函数的真实代码。它们是为对象添加上传图像的 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;
}
希望这对您有所帮助:)
我正在向 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
}
我现在需要添加一个新的函数,下面完全一样,但是代码是唯一的。
以下是我想到的事情:
- 使用回调函数作为参数,不知道怎么做。
- 使用普通代码的单个函数,return一个布尔值。如果为真,则在每个函数中执行唯一代码。这对我来说似乎不够优雅。
- 对唯一代码使用抽象class和模板抽象方法,但我不确定如何传递 $differentObject。
- 我什至怀疑这是不是装饰器模式的问题,但我从来没有这样使用过。
正确的模式是什么?
更新:
下面是其中两个函数的真实代码。它们是为对象添加上传图像的 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;
}
希望这对您有所帮助:)