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 Jetpack_Signature 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 Jetpack_Signature, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
6 | class Jetpack_Signature { |
||
7 | public $token; |
||
8 | public $secret; |
||
9 | |||
10 | function __construct( $access_token, $time_diff = 0 ) { |
||
19 | |||
20 | function sign_current_request( $override = array() ) { |
||
62 | |||
63 | // body_hash v. body-hash is annoying. Refactor to accept an array? |
||
64 | function sign_request( $token = '', $timestamp = 0, $nonce = '', $body_hash = '', $method = '', $url = '', $body = null, $verify_body_hash = true ) { |
||
65 | if ( !$this->secret ) { |
||
66 | return new Jetpack_Error( 'invalid_secret', 'Invalid secret' ); |
||
67 | } |
||
68 | |||
69 | if ( !$this->token ) { |
||
70 | return new Jetpack_Error( 'invalid_token', 'Invalid token' ); |
||
71 | } |
||
72 | |||
73 | list( $token ) = explode( '.', $token ); |
||
74 | |||
75 | if ( 0 !== strpos( $token, "$this->token:" ) ) { |
||
76 | return new Jetpack_Error( 'token_mismatch', 'Incorrect token' ); |
||
77 | } |
||
78 | |||
79 | $required_parameters = array( 'token', 'timestamp', 'nonce', 'method', 'url' ); |
||
80 | View Code Duplication | if ( !is_null( $body ) ) { |
|
81 | $required_parameters[] = 'body_hash'; |
||
82 | if ( !is_string( $body ) ) { |
||
83 | return new Jetpack_Error( 'invalid_body', 'Body is malformed.' ); |
||
84 | } |
||
85 | } |
||
86 | |||
87 | foreach ( $required_parameters as $required ) { |
||
88 | View Code Duplication | if ( !is_scalar( $$required ) ) { |
|
89 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', str_replace( '_', '-', $required ) ) ); |
||
90 | } |
||
91 | |||
92 | View Code Duplication | if ( !strlen( $$required ) ) { |
|
93 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is missing.', str_replace( '_', '-', $required ) ) ); |
||
94 | } |
||
95 | } |
||
96 | |||
97 | if ( is_null( $body ) ) { |
||
98 | if ( $body_hash ) { |
||
99 | return new Jetpack_Error( 'invalid_body_hash', 'The body hash does not match.' ); |
||
100 | } |
||
101 | } else { |
||
102 | if ( $verify_body_hash && jetpack_sha1_base64( $body ) !== $body_hash ) { |
||
103 | return new Jetpack_Error( 'invalid_body_hash', 'The body hash does not match.' ); |
||
104 | } |
||
105 | } |
||
106 | |||
107 | $parsed = parse_url( $url ); |
||
108 | if ( !isset( $parsed['host'] ) ) { |
||
109 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', 'url' ) ); |
||
110 | } |
||
111 | |||
112 | if ( $parsed['host'] === JETPACK__WPCOM_JSON_API_HOST ) { |
||
113 | $parsed['host'] = 'public-api.wordpress.com'; |
||
114 | } |
||
115 | |||
116 | if ( !empty( $parsed['port'] ) ) { |
||
117 | $port = $parsed['port']; |
||
118 | } else { |
||
119 | if ( 'http' == $parsed['scheme'] ) { |
||
120 | $port = 80; |
||
121 | } else if ( 'https' == $parsed['scheme'] ) { |
||
122 | $port = 443; |
||
123 | } else { |
||
124 | return new Jetpack_Error( 'unknown_scheme_port', "The scheme's port is unknown" ); |
||
125 | } |
||
126 | } |
||
127 | |||
128 | if ( !ctype_digit( "$timestamp" ) || 10 < strlen( $timestamp ) ) { // If Jetpack is around in 275 years, you can blame mdawaffe for the bug. |
||
129 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', 'timestamp' ) ); |
||
130 | } |
||
131 | |||
132 | $local_time = $timestamp - $this->time_diff; |
||
133 | if ( $local_time < time() - 600 || $local_time > time() + 300 ) { |
||
134 | return new Jetpack_Error( 'invalid_signature', 'The timestamp is too old.' ); |
||
135 | } |
||
136 | |||
137 | if ( 12 < strlen( $nonce ) || preg_match( '/[^a-zA-Z0-9]/', $nonce ) ) { |
||
138 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', 'nonce' ) ); |
||
139 | } |
||
140 | |||
141 | $normalized_request_pieces = array( |
||
142 | $token, |
||
143 | $timestamp, |
||
144 | $nonce, |
||
145 | $body_hash, |
||
146 | strtoupper( $method ), |
||
147 | strtolower( $parsed['host'] ), |
||
148 | $port, |
||
149 | $parsed['path'], |
||
150 | // Normalized Query String |
||
151 | ); |
||
152 | |||
153 | $normalized_request_pieces = array_merge( $normalized_request_pieces, $this->normalized_query_parameters( isset( $parsed['query'] ) ? $parsed['query'] : '' ) ); |
||
154 | |||
155 | $normalized_request_string = join( "\n", $normalized_request_pieces ) . "\n"; |
||
156 | |||
157 | return base64_encode( hash_hmac( 'sha1', $normalized_request_string, $this->secret, true ) ); |
||
158 | } |
||
159 | |||
160 | function normalized_query_parameters( $query_string ) { |
||
179 | |||
180 | function encode_3986( $string ) { |
||
184 | |||
185 | function join_with_equal_sign( $name, $value ) { |
||
188 | } |
||
189 | |||
193 |
PHP has two types of connecting operators (logical operators, and boolean operators):
and
&&
or
||
The difference between these is the order in which they are executed. In most cases, you would want to use a boolean operator like
&&
, or||
.Let’s take a look at a few examples:
Logical Operators are used for Control-Flow
One case where you explicitly want to use logical operators is for control-flow such as this:
Since
die
introduces problems of its own, f.e. it makes our code hardly testable, and prevents any kind of more sophisticated error handling; you probably do not want to use this in real-world code. Unfortunately, logical operators cannot be combined withthrow
at this point:These limitations lead to logical operators rarely being of use in current PHP code.