翻译自文章 Examples Of Refactoring PHP Code For Better Readability
重构代码是指当你重构已有代码时不改变其外部行为。更简单的说你的目标是在不改变原有功能的前提下把"坏"的代码变好。
网上有许多关于重构代码的指南。但是,我发现很多人只是向你介绍了写好代码和结构的思想,并没有向你展示如何去把你的"坏的"代码重构成"好的"代码。他们可能会谈及比较高级的理论像可读性、扩展性、稳定性、易于测试、减少复杂性之类的,可是这些都是重构过程中要达到的目标,他们并没有向我们展示实际中这些情况的例子是怎么样的。
在这篇文章中我不谈论你什么时候需要重构(个人认为你需要做这些当你遇到坏代码时),我也不谈论为什么我们需要重构。我只想关注一些在重构代码中会遇到的,共同的、可以上手的原理,我也会给你一些像真实情况的代码例子。在本文中我会使用php代码(WordPress是用php写的)但是这些原理你可以应用在任何语言。
不要重复你自己(DRY)
可能你最经常听到的编程规范就是 不要重复你自己(DRY)
。如果你发现你自己把相同的代码重复了几次,那你就应该把功能封装到它所属的类或方法中然后使用这个类或方法避免重复的代码。这意味着当你几个月后代码出了问题只需要修改一个地方的代码就可以了。
一个很好的例子是,当两个不同但相似的类需要一些相同的功能,你应该创建一个 abstract class
然后让这两个类继承这个abstract class
而不是在两个类当中重复代码。
修改前例子:
<?php
class AwesomeAddon {
private $settings;
public function __construct( $settings ) {
$this->set_settings( $settings );
}
protected function set_settings( $settings ) {
if ( ! is_array( $settings ) ) {
throw new \Exception( 'Invalid settings' );
}
$this->settings = $settings;
}
protected function do_something_awesome() {
//...
}
}
class EvenMoreAwesomeAddon {
private $settings;
public function __construct( $settings ) {
$this->set_settings( $settings );
}
protected function set_settings( $settings ) {
if ( ! is_array( $settings ) ) {
throw new \Exception( 'Invalid settings' );
}
$this->settings = $settings;
}
protected function do_something_even_more_awesome() {
//...
}
}
修改后代码:
<?php
abstract class Addon {
protected $settings;
protected function set_settings( $settings ) {
if ( ! is_array( $settings ) ) {
throw new \Exception( 'Invalid settings' );
}
$this->settings = $settings;
}
}
class AwesomeAddon extends Addon {
public function __construct( $settings ) {
$this->set_settings( $settings );
}
protected function do_something_awesome() {
//...
}
}
class EvenMoreAwesomeAddon extends Addon {
public function __construct( $settings ) {
$this->set_settings( $settings );
}
protected function do_something_even_more_awesome() {
//...
}
}
这只是个简单的关于构造代码避免重复的例子。
分解复杂函数
复杂的函数或方法是编程中另一个会导致技术负债和难以阅读的原因。
"程序是写给人看的,然后顺便让机器执行" —— Harold Abelson
让别人能读懂和理解你写的代码是最重要的事,所以让复杂函数变得好理解的方式是把它拆成更小的、更容易理解的小块。
下面是一个复杂的函数。不要担心你看不懂他在写什么,你只要看一眼他有多复杂就可以了。
<?php
function upload_attachment_to_s3( $post_id, $data = null, $file_path = null, $force_new_s3_client = false, $remove_local_files = true ) {
$return_metadata = null;
if ( is_null( $data ) ) {
$data = wp_get_attachment_metadata( $post_id, true );
} else {
// As we have passed in the meta, return it later
$return_metadata = $data;
}
if ( is_wp_error( $data ) ) {
return $data;
}
// Allow S3 upload to be hijacked / cancelled for any reason
$pre = apply_filters( 'as3cf_pre_upload_attachment', false, $post_id, $data );
if ( false !== $pre ) {
if ( ! is_null( $return_metadata ) ) {
// If the attachment metadata is supplied, return it
return $data;
}
$error_msg = is_string( $pre ) ? $pre : __( 'Upload aborted by filter \'as3cf_pre_upload_attachment\'', 'amazon-s3-and-cloudfront' );
return $this->return_upload_error( $error_msg );
}
if ( is_null( $file_path ) ) {
$file_path = get_attached_file( $post_id, true );
}
// Check file exists locally before attempting upload
if ( ! file_exists( $file_path ) ) {
$error_msg = sprintf( __( 'File %s does not exist', 'amazon-s3-and-cloudfront' ), $file_path );
return $this->return_upload_error( $error_msg, $return_metadata );
}
$file_name = basename( $file_path );
$type = get_post_mime_type( $post_id );
$allowed_types = $this->get_allowed_mime_types();
// check mime type of file is in allowed S3 mime types
if ( ! in_array( $type, $allowed_types ) ) {
$error_msg = sprintf( __( 'Mime type %s is not allowed', 'amazon-s3-and-cloudfront' ), $type );
return $this->return_upload_error( $error_msg, $return_metadata );
}
$acl = self::DEFAULT_ACL;
// check the attachment already exists in S3, eg. edit or restore image
if ( ( $old_s3object = $this->get_attachment_s3_info( $post_id ) ) ) {
// use existing non default ACL if attachment already exists
if ( isset( $old_s3object['acl'] ) ) {
$acl = $old_s3object['acl'];
}
// use existing prefix
$prefix = dirname( $old_s3object['key'] );
$prefix = ( '.' === $prefix ) ? '' : $prefix . '/';
// use existing bucket
$bucket = $old_s3object['bucket'];
// get existing region
if ( isset( $old_s3object['region'] ) ) {
$region = $old_s3object['region'];
};
} else {
// derive prefix from various settings
if ( isset( $data['file'] ) ) {
$time = $this->get_folder_time_from_url( $data['file'] );
} else {
$time = $this->get_attachment_folder_time( $post_id );
$time = date( 'Y/m', $time );
}
$prefix = $this->get_file_prefix( $time );
// use bucket from settings
$bucket = $this->get_setting( 'bucket' );
$region = $this->get_setting( 'region' );
if ( is_wp_error( $region ) ) {
$region = '';
}
}
$acl = apply_filters( 'as3cf_upload_acl', $acl, $data, $post_id );
$s3object = array(
'bucket' => $bucket,
'key' => $prefix . $file_name,
'region' => $region,
);
// store acl if not default
if ( $acl != self::DEFAULT_ACL ) {
$s3object['acl'] = $acl;
}
$s3client = $this->get_s3client( $region, $force_new_s3_client );
$args = array(
'Bucket' => $bucket,
'Key' => $prefix . $file_name,
'SourceFile' => $file_path,
'ACL' => $acl,
'ContentType' => $type,
'CacheControl' => 'max-age=31536000',
'Expires' => date( 'D, d M Y H:i:s O', time() + 31536000 ),
);
$args = apply_filters( 'as3cf_object_meta', $args, $post_id );
$files_to_remove = array();
if ( file_exists( $file_path ) ) {
$files_to_remove[] = $file_path;
try {
$s3client->putObject( $args );
} catch ( Exception $e ) {
$error_msg = sprintf( __( 'Error uploading %s to S3: %s', 'amazon-s3-and-cloudfront' ), $file_path, $e->getMessage() );
return $this->return_upload_error( $error_msg, $return_metadata );
}
}
delete_post_meta( $post_id, 'amazonS3_info' );
add_post_meta( $post_id, 'amazonS3_info', $s3object );
$file_paths = $this->get_attachment_file_paths( $post_id, true, $data );
$additional_images = array();
$filesize_total = 0;
$remove_local_files_setting = $this->get_setting( 'remove-local-file' );
if ( $remove_local_files_setting ) {
$bytes = filesize( $file_path );
if ( false !== $bytes ) {
// Store in the attachment meta data for use by WP
$data['filesize'] = $bytes;
if ( is_null( $return_metadata ) ) {
// Update metadata with filesize
update_post_meta( $post_id, '_wp_attachment_metadata', $data );
}
// Add to the file size total
$filesize_total += $bytes;
}
}
foreach ( $file_paths as $file_path ) {
if ( ! in_array( $file_path, $files_to_remove ) ) {
$additional_images[] = array(
'Key' => $prefix . basename( $file_path ),
'SourceFile' => $file_path,
);
$files_to_remove[] = $file_path;
if ( $remove_local_files_setting ) {
// Record the file size for the additional image
$bytes = filesize( $file_path );
if ( false !== $bytes ) {
$filesize_total += $bytes;
}
}
}
}
if ( $remove_local_files ) {
if ( $remove_local_files_setting ) {
// Allow other functions to remove files after they have processed
$files_to_remove = apply_filters( 'as3cf_upload_attachment_local_files_to_remove', $files_to_remove, $post_id, $file_path );
// Remove duplicates
$files_to_remove = array_unique( $files_to_remove );
// Delete the files
$this->remove_local_files( $files_to_remove );
}
}
// Store the file size in the attachment meta if we are removing local file
if ( $remove_local_files_setting ) {
if ( $filesize_total > 0 ) {
// Add the total file size for all image sizes
update_post_meta( $post_id, 'wpos3_filesize_total', $filesize_total );
}
} else {
if ( isset( $data['filesize'] ) ) {
// Make sure we don't have a cached file sizes in the meta
unset( $data['filesize'] );
if ( is_null( $return_metadata ) ) {
// Remove the filesize from the metadata
update_post_meta( $post_id, '_wp_attachment_metadata', $data );
}
delete_post_meta( $post_id, 'wpos3_filesize_total' );
}
}
if ( ! is_null( $return_metadata ) ) {
// If the attachment metadata is supplied, return it
return $data;
}
return $s3object;
}
如果整个函数变成下面这样会更好理解
<?php
function upload_attachment_to_s3( $post_id, $data = null, $file_path = null, $force_new_s3_client = false, $remove_local_files = true ) {
$return_metadata = $this->get_attachment_metadata( $post_id );
if ( is_wp_error( $return_metadata ) ) {
return $return_metadata;
}
// Allow S3 upload to be hijacked / cancelled for any reason
$pre = apply_filters( 'as3cf_pre_upload_attachment', false, $post_id, $data );
if ( $this->upload_should_be_cancelled( $pre ) ) {
return $pre;
}
// Check file exists locally before attempting upload
if ( ! $this->local_file_exists() ) {
$error_msg = sprintf( __( 'File %s does not exist', 'amazon-s3-and-cloudfront' ), $file_path );
return $this->return_upload_error( $error_msg, $return_metadata );
}
// check mime type of file is in allowed S3 mime types
if ( ! $this->is_valid_mime_type() ) {
$error_msg = sprintf( __( 'Mime type %s is not allowed', 'amazon-s3-and-cloudfront' ), $type );
return $this->return_upload_error( $error_msg, $return_metadata );
}
$s3object = $this->get_attachment_s3_info( $post_id );
$acl = $this->get_s3object_acl( $s3object );
$s3client = $this->get_s3client( $region, $force_new_s3_client );
$args = array(
'Bucket' => $s3object['bucket'],
'Key' => $s3object['key'],
'SourceFile' => $s3object['source_file'],
'ACL' => $acl,
'ContentType' => $s3object['mime_type'],
'CacheControl' => 'max-age=31536000',
'Expires' => date( 'D, d M Y H:i:s O', time() + 31536000 ),
);
$s3client->putObject( $args );
$this->maybe_remove_files( $args, $s3object );
return $s3object;
}
这是很好阅读和理解的。简单的把大块的代码分成小的代码库是很好的。
有个事情需要记住的是,不要担心你用很长的名字当方法名,记住你的目标是可读性,所以如果只用简单的名字命名的话它会让你的代码变得很难以理解。举个例子:
$this->get_att_inf( $post_id );
比下面的代码难以理解:
$this->get_attachment_s3_info( $post_id );
分解复杂的条件
你应该见过像下面这样条件很长的例子:
<?php
if ( isset( $settings['wp-uploads'] ) && $settings['wp-uploads'] && in_array( $key, array( 'copy-to-s3', 'serve-from-s3' ) ) ) {
return '1';
}
带条件的长段代码很难阅读和理解。一个简单的解决方案是将条件代码提取为明确命名的方法。例如:
<?php
if ( upload_is_valid( $settings, $key ) ) {
return '1';
}
function upload_is_valid( $settings, $key ) {
return isset( $settings['wp-uploads'] ) && $settings['wp-uploads'] && in_array( $key, array( 'copy-to-s3', 'serve-from-s3' ) );
}
这使得你的代码对于以后的维护者来说更容易理解。只要条件方法是明确命名的,不用看代码也很容易理解它是做什么的。这种做法也被称为声明式编程。
用守卫子句GUARD CLAUSES
替代嵌套条件
另一种重构复杂条件的方法是使用所谓的“守卫子句”。 Guard子句简单地提取所有导致调用异常或立即从方法返回值的条件,把它放在方法的开始位置。例如:
<?php
function get_setting( $key, $default = '' ) {
$settings = $this->get_settings();
// If legacy setting set, migrate settings
if ( isset( $settings['wp-uploads'] ) && $settings['wp-uploads'] && in_array( $key, array( 'copy-to-s3', 'serve-from-s3' ) ) ) {
return $default;
} else {
// Turn on object versioning by default
if ( 'object-versioning' == $key && ! isset( $settings['object-versioning'] ) ) {
return $default;
} else {
// Default object prefix
if ( 'object-prefix' == $key && ! isset( $settings['object-prefix'] ) ) {
return $this->get_default_object_prefix();
} else {
if ( 'use-yearmonth-folders' == $key && ! isset( $settings['use-yearmonth-folders'] ) ) {
return get_option( 'uploads_use_yearmonth_folders' );
} else {
$value = parent::get_setting( $key, $default );
return apply_filters( 'as3cf_setting_' . $key, $value );
}
}
}
}
return $default;
}
这里你可以看到如果方法变得更复杂你可以多快结束"条件地狱"。但是如果你使用守卫子句来重构方法,它将变成下面这样:
<?php
function get_setting( $key, $default = '' ) {
$settings = $this->get_settings();
// If legacy setting set, migrate settings
if ( isset( $settings['wp-uploads'] ) && $settings['wp-uploads'] && in_array( $key, array( 'copy-to-s3', 'serve-from-s3' ) ) ) {
return $default;
}
// Turn on object versioning by default
if ( 'object-versioning' == $key && ! isset( $settings['object-versioning'] ) ) {
return $default;
}
// Default object prefix
if ( 'object-prefix' == $key && ! isset( $settings['object-prefix'] ) ) {
return $this->get_default_object_prefix();
}
// Default use year and month folders
if ( 'use-yearmonth-folders' == $key && ! isset( $settings['use-yearmonth-folders'] ) ) {
return get_option( 'uploads_use_yearmonth_folders' );
}
$value = parent::get_setting( $key, $default );
return apply_filters( 'as3cf_setting_' . $key, $value );
}
现在即使方法变得复杂,一段时间后他也不会变成维护的难题。
使用函数方法重构循环和条件
这是一个比较高级的重构方法,它被大量使用在函数式编程和类库中(这种类型的方法在javascript领域中经常使用)。你可能有听说过map
和reduce
也想知道他们是什么并且如何使用,事实证明这些方法可以大幅度提高你代码的可读性。
这个例子的灵感来自Adam Wathan关于这个主题的很棒的视频(你应该看看他即将出版的书),基于Laravel Collections。不过,我已经调整了该示例,基于标准PHP函数。
让我们看看两个常见的场景,并看看如何使用函数方法来改进它们。我们的示例从API获取一堆$events
,然后根据事件类型计算得分:
<?php
$events = file_get_contents( 'https://someapi.com/events' );
$types = array();
foreach ( $events as $event ) {
$types[] = $event->type;
}
$score = 0;
foreach ( $types as $type ) {
switch ( $type ) {
case 'type1':
$score += 2;
break;
case 'type2':
$score += 5;
break;
case 'type3':
$score += 10;
break;
default:
$score += 1;
break;
}
}
我们可以改进的第一件事是用map
函数替换foreach
循环。当你想从现有的数组中创建一个新的数组时,可以使用map
函数。在我们的例子中,我们从$events
数组创建一个$ types
数组。 PHP有一个array_map
函数,可以让我们把上面的第一个foreach
编写成如下所示:
<?php
$types = array_map( function( $event ) {
return $event->type;
}, $events );
提示:要使用匿名函数,需要的php版本为,PHP 5.3+
我们可以做的第二件事情是,把大的switch
语句分解,让它更简单的进行下去
<?php
$scores = array(
'type1' => 2,
'type2' => 5,
'type3' => 10,
);
$score = 0;
foreach ( $types as $type ) {
$score += isset( $scores[$type] ) ? $scores[$type] : 1;
}
实际上我们可以走的更远一步,通过php的 array_reduce
方法在单个方法内计算$score
的值。一个reduce
方法接收一个数组的值然后把他们归成一个值:
<?php
$scores = array(
'type1' => 2,
'type2' => 5,
'type3' => 10,
);
$score = array_reduce( $types, function( $result, $type ) use ( $scores ) {
return $result += isset( $scores[$type] ) ? $scores[$type] : 1;
} );
把现在有的代码合并起来
<?php
$events = file_get_contents( 'https://someapi.com/events' );
$types = array_map( function( $event ) {
return $event->type;
}, $events );
$scores = array(
'type1' => 2,
'type2' => 5,
'type3' => 10,
);
$score = array_reduce( $types, function( $result, $type ) use ( $scores ) {
return $result += isset( $scores[$type] ) ? $scores[$type] : 1;
} );
好多了。看不到循环或则条件了。你可以想象,如果获取$types
和计算$score
变得更加复杂的话,那么在调用的方法里面重构map
和reduce
方法是比较容易的。现在可以这样做了,我们已经降低复杂度到一个函数了。
进一步阅读
我只是在这篇文章中讲了重构的表面知识。还有很多我可以谈论的内容,但是希望这可以让你对重构代码的实际情况有个小的了解。
如果你想深入了解,我推荐SourceMaking重构指南。它涵盖了大量主题,并且每个主题都有一个示例,因此您可以准确了解重构。
网友评论