Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like TAV_Remote_Notification_Client often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use TAV_Remote_Notification_Client, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 27 | class TAV_Remote_Notification_Client { |
||
| 28 | |||
| 29 | /** |
||
| 30 | * Channel ID |
||
| 31 | * |
||
| 32 | * @var int |
||
| 33 | */ |
||
| 34 | protected $id; |
||
| 35 | |||
| 36 | /** |
||
| 37 | * Channel identification key |
||
| 38 | * |
||
| 39 | * @var string |
||
| 40 | */ |
||
| 41 | protected $key; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * Notice unique identifier |
||
| 45 | * |
||
| 46 | * @var string |
||
| 47 | */ |
||
| 48 | protected $notice_id; |
||
| 49 | |||
| 50 | /** |
||
| 51 | * Notification server URL |
||
| 52 | * |
||
| 53 | * @var string |
||
| 54 | */ |
||
| 55 | protected $server; |
||
| 56 | |||
| 57 | /** |
||
| 58 | * Notification caching delay |
||
| 59 | * |
||
| 60 | * @var int |
||
| 61 | */ |
||
| 62 | protected $cache; |
||
| 63 | |||
| 64 | /** |
||
| 65 | * Error message |
||
| 66 | * |
||
| 67 | * @var string |
||
| 68 | */ |
||
| 69 | protected $error; |
||
| 70 | |||
| 71 | /** |
||
| 72 | * Notification |
||
| 73 | * |
||
| 74 | * @var string|object |
||
| 75 | */ |
||
| 76 | protected $notice; |
||
| 77 | |||
| 78 | /** |
||
| 79 | * Class version. |
||
| 80 | * |
||
| 81 | * @since 0.1.0 |
||
| 82 | * |
||
| 83 | * @var string |
||
| 84 | */ |
||
| 85 | protected static $version = '0.2.0'; |
||
| 86 | |||
| 87 | public function __construct( $channel_id = false, $channel_key = false, $server = false ) { |
||
| 109 | |||
| 110 | /** |
||
| 111 | * Instantiate the plugin |
||
| 112 | * |
||
| 113 | * @since 1.2.0 |
||
| 114 | * @return void |
||
| 115 | */ |
||
| 116 | public function init() { |
||
| 127 | |||
| 128 | /** |
||
| 129 | * Get the notification message |
||
| 130 | * |
||
| 131 | * @since 1.2.0 |
||
| 132 | * @return string |
||
| 133 | */ |
||
| 134 | public function get_notice() { |
||
| 143 | |||
| 144 | /** |
||
| 145 | * Retrieve the notice from the transient or from the remote server |
||
| 146 | * |
||
| 147 | * @since 1.2.0 |
||
| 148 | * @return mixed |
||
| 149 | */ |
||
| 150 | protected function fetch_notice() { |
||
| 161 | |||
| 162 | /** |
||
| 163 | * Get the remote server URL |
||
| 164 | * |
||
| 165 | * @since 1.2.0 |
||
| 166 | * @return string |
||
| 167 | */ |
||
| 168 | protected function get_remote_url() { |
||
| 175 | |||
| 176 | /** |
||
| 177 | * Maybe get a notification from the remote server |
||
| 178 | * |
||
| 179 | * @since 1.2.0 |
||
| 180 | * @return string|WP_Error |
||
| 181 | */ |
||
| 182 | protected function remote_get_notice() { |
||
| 217 | |||
| 218 | /** |
||
| 219 | * Check if the notification returned by the server is an error |
||
| 220 | * |
||
| 221 | * @since 1.2.0 |
||
| 222 | * |
||
| 223 | * @param object $notification Notification returned |
||
| 224 | * |
||
| 225 | * @return bool |
||
| 226 | */ |
||
| 227 | protected function is_notification_error( $notification ) { |
||
| 236 | |||
| 237 | /** |
||
| 238 | * Get the error message returned by the remote server |
||
| 239 | * |
||
| 240 | * @since 1.2.0 |
||
| 241 | * |
||
| 242 | * @param object $notification Notification returned |
||
| 243 | * |
||
| 244 | * @return bool|string |
||
| 245 | */ |
||
| 246 | protected function get_notification_error_message( $notification ) { |
||
| 259 | |||
| 260 | /** |
||
| 261 | * Get the payload required for querying the remote server |
||
| 262 | * |
||
| 263 | * @since 1.2.0 |
||
| 264 | * @return string |
||
| 265 | */ |
||
| 266 | protected function get_payload() { |
||
| 269 | |||
| 270 | /** |
||
| 271 | * Get the full URL used for the remote get |
||
| 272 | * |
||
| 273 | * @since 1.2.0 |
||
| 274 | * @return string |
||
| 275 | */ |
||
| 276 | protected function build_query_url() { |
||
| 279 | |||
| 280 | /** |
||
| 281 | * Check if the notification has been dismissed |
||
| 282 | * |
||
| 283 | * @since 1.2.0 |
||
| 284 | * @return bool |
||
| 285 | */ |
||
| 286 | protected function is_notification_dismissed() { |
||
| 303 | |||
| 304 | /** |
||
| 305 | * Check if the notification can be displayed for the current post type |
||
| 306 | * |
||
| 307 | * @since 1.2.0 |
||
| 308 | * @return bool |
||
| 309 | */ |
||
| 310 | protected function is_post_type_restricted() { |
||
| 332 | |||
| 333 | /** |
||
| 334 | * Check if the notification has started yet |
||
| 335 | * |
||
| 336 | * @since 1.2.0 |
||
| 337 | * @return bool |
||
| 338 | */ |
||
| 339 | View Code Duplication | protected function is_notification_started() { |
|
| 348 | |||
| 349 | /** |
||
| 350 | * Check if the notification has expired |
||
| 351 | * |
||
| 352 | * @since 1.2.0 |
||
| 353 | * @return bool |
||
| 354 | */ |
||
| 355 | View Code Duplication | protected function has_notification_ended() { |
|
| 364 | |||
| 365 | /** |
||
| 366 | * Display the admin notice |
||
| 367 | * |
||
| 368 | * The function will do some checks to verify if |
||
| 369 | * the notice can be displayed on the current page. |
||
| 370 | * If all the checks are passed, the notice |
||
| 371 | * is added to the page. |
||
| 372 | * |
||
| 373 | * @since 0.1.0 |
||
| 374 | */ |
||
| 375 | public function show_notice() { |
||
| 448 | |||
| 449 | /** |
||
| 450 | * Dismiss notice |
||
| 451 | * |
||
| 452 | * When the user dismisses a notice, its slug |
||
| 453 | * is added to the _rn_dismissed entry in the DB options table. |
||
| 454 | * This entry is then used to check if a notie has been dismissed |
||
| 455 | * before displaying it on the dashboard. |
||
| 456 | * |
||
| 457 | * @since 0.1.0 |
||
| 458 | */ |
||
| 459 | public function dismiss() { |
||
| 504 | |||
| 505 | /** |
||
| 506 | * Adds inline style for non standard notices |
||
| 507 | * |
||
| 508 | * This function will only be called if the notice style is not standard. |
||
| 509 | * |
||
| 510 | * @since 0.1.0 |
||
| 511 | */ |
||
| 512 | public function style() { ?> |
||
| 517 | |||
| 518 | /** |
||
| 519 | * Dismiss notice using Ajax |
||
| 520 | * |
||
| 521 | * This function is NOT used. Testing only. |
||
| 522 | */ |
||
| 523 | public function script() { |
||
| 551 | |||
| 552 | /** |
||
| 553 | * Debug info. |
||
| 554 | * |
||
| 555 | * Display an error message commented in the admin footer. |
||
| 556 | * |
||
| 557 | * @since 0.1.2 |
||
| 558 | */ |
||
| 559 | public function debug_info() { |
||
| 566 | |||
| 567 | } |
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.