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 OpenWeatherMap 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 OpenWeatherMap, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
34 | class OpenWeatherMap |
||
35 | { |
||
36 | /** |
||
37 | * The copyright notice. This is no official text, it was created by |
||
38 | * following the guidelines at http://openweathermap.org/copyright. |
||
39 | * |
||
40 | * @var string $copyright |
||
41 | */ |
||
42 | const COPYRIGHT = "Weather data from <a href=\"http://www.openweathermap.org\">OpenWeatherMap.org</a>"; |
||
43 | |||
44 | /** |
||
45 | * @var string The basic api url to fetch weather data from. |
||
46 | */ |
||
47 | private $weatherUrl = 'http://api.openweathermap.org/data/2.5/weather?'; |
||
48 | |||
49 | /** |
||
50 | * @var string The basic api url to fetch weekly forecast data from. |
||
51 | */ |
||
52 | private $weatherHourlyForecastUrl = 'http://api.openweathermap.org/data/2.5/forecast?'; |
||
53 | |||
54 | /** |
||
55 | * @var string The basic api url to fetch daily forecast data from. |
||
56 | */ |
||
57 | private $weatherDailyForecastUrl = 'http://api.openweathermap.org/data/2.5/forecast/daily?'; |
||
58 | |||
59 | /** |
||
60 | * @var string The basic api url to fetch history weather data from. |
||
61 | */ |
||
62 | private $weatherHistoryUrl = 'http://api.openweathermap.org/data/2.5/history/city?'; |
||
63 | |||
64 | /** |
||
65 | * @var AbstractCache|bool $cache The cache to use. |
||
66 | */ |
||
67 | private $cache = false; |
||
68 | |||
69 | /** |
||
70 | * @var int |
||
71 | */ |
||
72 | private $seconds; |
||
73 | |||
74 | /** |
||
75 | * @var bool |
||
76 | */ |
||
77 | private $wasCached = false; |
||
78 | |||
79 | /** |
||
80 | * @var FetcherInterface The url fetcher. |
||
81 | */ |
||
82 | private $fetcher; |
||
83 | |||
84 | /** |
||
85 | * @var string |
||
86 | */ |
||
87 | private $apiKey = ''; |
||
88 | |||
89 | /** |
||
90 | * Constructs the OpenWeatherMap object. |
||
91 | * |
||
92 | * @param string $apiKey The OpenWeatherMap API key. Required and only optional for BC. |
||
93 | * @param null|FetcherInterface $fetcher The interface to fetch the data from OpenWeatherMap. Defaults to |
||
94 | * CurlFetcher() if cURL is available. Otherwise defaults to |
||
95 | * FileGetContentsFetcher() using 'file_get_contents()'. |
||
96 | * @param bool|string $cache If set to false, caching is disabled. Otherwise this must be a class |
||
97 | * extending AbstractCache. Defaults to false. |
||
98 | * @param int $seconds How long weather data shall be cached. Default 10 minutes. |
||
99 | * |
||
100 | * @throws \Exception If $cache is neither false nor a valid callable extending Cmfcmf\OpenWeatherMap\Util\Cache. |
||
101 | * |
||
102 | * @api |
||
103 | */ |
||
104 | 7 | public function __construct($apiKey = '', $fetcher = null, $cache = false, $seconds = 600) |
|
132 | |||
133 | /** |
||
134 | * Sets the API Key. |
||
135 | * |
||
136 | * @param string $apiKey API key for the OpenWeatherMap account. |
||
137 | * |
||
138 | * @api |
||
139 | */ |
||
140 | 6 | public function setApiKey($apiKey) |
|
144 | |||
145 | /** |
||
146 | * Returns the API Key. |
||
147 | * |
||
148 | * @return string |
||
149 | * |
||
150 | * @api |
||
151 | */ |
||
152 | public function getApiKey() |
||
156 | |||
157 | /** |
||
158 | * Returns the current weather at the place you specified. |
||
159 | * |
||
160 | * @param array|int|string $query The place to get weather information for. For possible values see below. |
||
161 | * @param string $units Can be either 'metric' or 'imperial' (default). This affects almost all units returned. |
||
162 | * @param string $lang The language to use for descriptions, default is 'en'. For possible values see http://openweathermap.org/current#multi. |
||
163 | * @param string $appid Your app id, default ''. See http://openweathermap.org/appid for more details. |
||
164 | * |
||
165 | * @throws OpenWeatherMap\Exception If OpenWeatherMap returns an error. |
||
166 | * @throws \InvalidArgumentException If an argument error occurs. |
||
167 | * |
||
168 | * @return CurrentWeather The weather object. |
||
169 | * |
||
170 | * There are three ways to specify the place to get weather information for: |
||
171 | * - Use the city name: $query must be a string containing the city name. |
||
172 | * - Use the city id: $query must be an integer containing the city id. |
||
173 | * - Use the coordinates: $query must be an associative array containing the 'lat' and 'lon' values. |
||
174 | * |
||
175 | * @api |
||
176 | */ |
||
177 | 6 | public function getWeather($query, $units = 'imperial', $lang = 'en', $appid = '') |
|
184 | |||
185 | /** |
||
186 | * Returns the forecast for the place you specified. DANGER: Might return |
||
187 | * fewer results than requested due to a bug in the OpenWeatherMap API! |
||
188 | * |
||
189 | * @param array|int|string $query The place to get weather information for. For possible values see ::getWeather. |
||
190 | * @param string $units Can be either 'metric' or 'imperial' (default). This affects almost all units returned. |
||
191 | * @param string $lang The language to use for descriptions, default is 'en'. For possible values see http://openweathermap.org/current#multi. |
||
192 | * @param string $appid Your app id, default ''. See http://openweathermap.org/appid for more details. |
||
193 | * @param int $days For how much days you want to get a forecast. Default 1, maximum: 16. |
||
194 | * |
||
195 | * @throws OpenWeatherMap\Exception If OpenWeatherMap returns an error. |
||
196 | * @throws \InvalidArgumentException If an argument error occurs. |
||
197 | * |
||
198 | * @return WeatherForecast |
||
199 | * |
||
200 | * @api |
||
201 | */ |
||
202 | 1 | public function getWeatherForecast($query, $units = 'imperial', $lang = 'en', $appid = '', $days = 1) |
|
203 | { |
||
204 | 1 | if ($days <= 5) { |
|
205 | $answer = $this->getRawHourlyForecastData($query, $units, $lang, $appid, 'xml'); |
||
206 | 1 | } elseif ($days <= 16) { |
|
207 | 1 | $answer = $this->getRawDailyForecastData($query, $units, $lang, $appid, 'xml', $days); |
|
208 | 1 | } else { |
|
209 | throw new \InvalidArgumentException('Error: forecasts are only available for the next 16 days. $days must be 16 or lower.'); |
||
210 | } |
||
211 | 1 | $xml = $this->parseXML($answer); |
|
212 | |||
213 | 1 | return new WeatherForecast($xml, $units, $days); |
|
214 | } |
||
215 | |||
216 | /** |
||
217 | * Returns the weather history for the place you specified. |
||
218 | * |
||
219 | * @param array|int|string $query The place to get weather information for. For possible values see ::getWeather. |
||
220 | * @param \DateTime $start |
||
221 | * @param int $endOrCount |
||
222 | * @param string $type Can either be 'tick', 'hour' or 'day'. |
||
223 | * @param string $units Can be either 'metric' or 'imperial' (default). This affects almost all units returned. |
||
224 | * @param string $lang The language to use for descriptions, default is 'en'. For possible values see http://openweathermap.org/current#multi. |
||
225 | * @param string $appid Your app id, default ''. See http://openweathermap.org/appid for more details. |
||
226 | * |
||
227 | * @throws OpenWeatherMap\Exception If OpenWeatherMap returns an error. |
||
228 | * @throws \InvalidArgumentException If an argument error occurs. |
||
229 | * |
||
230 | * @return WeatherHistory |
||
231 | * |
||
232 | * @api |
||
233 | */ |
||
234 | public function getWeatherHistory($query, \DateTime $start, $endOrCount = 1, $type = 'hour', $units = 'imperial', $lang = 'en', $appid = '') |
||
248 | |||
249 | /** |
||
250 | * Directly returns the xml/json/html string returned by OpenWeatherMap for the current weather. |
||
251 | * |
||
252 | * @param array|int|string $query The place to get weather information for. For possible values see ::getWeather. |
||
253 | * @param string $units Can be either 'metric' or 'imperial' (default). This affects almost all units returned. |
||
254 | * @param string $lang The language to use for descriptions, default is 'en'. For possible values see http://openweathermap.org/current#multi. |
||
255 | * @param string $appid Your app id, default ''. See http://openweathermap.org/appid for more details. |
||
256 | * @param string $mode The format of the data fetched. Possible values are 'json', 'html' and 'xml' (default). |
||
257 | * |
||
258 | * @return string Returns false on failure and the fetched data in the format you specified on success. |
||
259 | * |
||
260 | * Warning: If an error occurs, OpenWeatherMap ALWAYS returns json data. |
||
261 | * |
||
262 | * @api |
||
263 | */ |
||
264 | 6 | public function getRawWeatherData($query, $units = 'imperial', $lang = 'en', $appid = '', $mode = 'xml') |
|
270 | |||
271 | /** |
||
272 | * Directly returns the xml/json/html string returned by OpenWeatherMap for the hourly forecast. |
||
273 | * |
||
274 | * @param array|int|string $query The place to get weather information for. For possible values see ::getWeather. |
||
275 | * @param string $units Can be either 'metric' or 'imperial' (default). This affects almost all units returned. |
||
276 | * @param string $lang The language to use for descriptions, default is 'en'. For possible values see http://openweathermap.org/current#multi. |
||
277 | * @param string $appid Your app id, default ''. See http://openweathermap.org/appid for more details. |
||
278 | * @param string $mode The format of the data fetched. Possible values are 'json', 'html' and 'xml' (default). |
||
279 | * |
||
280 | * @return string Returns false on failure and the fetched data in the format you specified on success. |
||
281 | * |
||
282 | * Warning: If an error occurs, OpenWeatherMap ALWAYS returns json data. |
||
283 | * |
||
284 | * @api |
||
285 | */ |
||
286 | public function getRawHourlyForecastData($query, $units = 'imperial', $lang = 'en', $appid = '', $mode = 'xml') |
||
292 | |||
293 | /** |
||
294 | * Directly returns the xml/json/html string returned by OpenWeatherMap for the daily forecast. |
||
295 | * |
||
296 | * @param array|int|string $query The place to get weather information for. For possible values see ::getWeather. |
||
297 | * @param string $units Can be either 'metric' or 'imperial' (default). This affects almost all units returned. |
||
298 | * @param string $lang The language to use for descriptions, default is 'en'. For possible values see http://openweathermap.org/current#multi. |
||
299 | * @param string $appid Your app id, default ''. See http://openweathermap.org/appid for more details. |
||
300 | * @param string $mode The format of the data fetched. Possible values are 'json', 'html' and 'xml' (default) |
||
301 | * @param int $cnt How many days of forecast shall be returned? Maximum (and default): 16 |
||
302 | * |
||
303 | * @throws \InvalidArgumentException If $cnt is higher than 16. |
||
304 | * |
||
305 | * @return string Returns false on failure and the fetched data in the format you specified on success. |
||
306 | * |
||
307 | * Warning: If an error occurs, OpenWeatherMap ALWAYS returns json data. |
||
308 | * |
||
309 | * @api |
||
310 | */ |
||
311 | 1 | public function getRawDailyForecastData($query, $units = 'imperial', $lang = 'en', $appid = '', $mode = 'xml', $cnt = 16) |
|
312 | { |
||
313 | 1 | if ($cnt > 16) { |
|
314 | throw new \InvalidArgumentException('$cnt must be 16 or lower!'); |
||
315 | } |
||
316 | 1 | $url = $this->buildUrl($query, $units, $lang, $appid, $mode, $this->weatherDailyForecastUrl) . "&cnt=$cnt"; |
|
317 | |||
318 | 1 | return $this->cacheOrFetchResult($url); |
|
319 | } |
||
320 | |||
321 | /** |
||
322 | * Directly returns the xml/json/html string returned by OpenWeatherMap for the weather history. |
||
323 | * |
||
324 | * @param array|int|string $query The place to get weather information for. For possible values see ::getWeather. |
||
325 | * @param \DateTime $start The \DateTime object of the date to get the first weather information from. |
||
326 | * @param \DateTime|int $endOrCount Can be either a \DateTime object representing the end of the period to |
||
327 | * receive weather history data for or an integer counting the number of |
||
328 | * reports requested. |
||
329 | * @param string $type The period of the weather history requested. Can be either be either "tick", |
||
330 | * "hour" or "day". |
||
331 | * @param string $units Can be either 'metric' or 'imperial' (default). This affects almost all units returned. |
||
332 | * @param string $lang The language to use for descriptions, default is 'en'. For possible values see http://openweathermap.org/current#multi. |
||
333 | * @param string $appid Your app id, default ''. See http://openweathermap.org/appid for more details. |
||
334 | * |
||
335 | * @throws \InvalidArgumentException |
||
336 | * |
||
337 | * @return string Returns false on failure and the fetched data in the format you specified on success. |
||
338 | * |
||
339 | * Warning If an error occurred, OpenWeatherMap ALWAYS returns data in json format. |
||
340 | * |
||
341 | * @api |
||
342 | */ |
||
343 | public function getRawWeatherHistory($query, \DateTime $start, $endOrCount = 1, $type = 'hour', $units = 'imperial', $lang = 'en', $appid = '') |
||
361 | |||
362 | /** |
||
363 | * Returns whether or not the last result was fetched from the cache. |
||
364 | * |
||
365 | * @return bool true if last result was fetched from cache, false otherwise. |
||
366 | */ |
||
367 | public function wasCached() |
||
371 | |||
372 | /** |
||
373 | * @deprecated Use {@link self::getRawWeatherData()} instead. |
||
374 | */ |
||
375 | public function getRawData($query, $units = 'imperial', $lang = 'en', $appid = '', $mode = 'xml') |
||
376 | { |
||
377 | return $this->getRawWeatherData($query, $units, $lang, $appid, $mode); |
||
378 | } |
||
379 | |||
380 | /** |
||
381 | * Fetches the result or delivers a cached version of the result. |
||
382 | * |
||
383 | * @param string $url |
||
384 | * |
||
385 | * @return string |
||
386 | */ |
||
387 | 7 | private function cacheOrFetchResult($url) |
|
406 | |||
407 | /** |
||
408 | * Build the url to fetch weather data from. |
||
409 | * |
||
410 | * @param $query |
||
411 | * @param $units |
||
412 | * @param $lang |
||
413 | * @param $appid |
||
414 | * @param $mode |
||
415 | * @param string $url The url to prepend. |
||
416 | * |
||
417 | * @return bool|string The fetched url, false on failure. |
||
418 | */ |
||
419 | 7 | private function buildUrl($query, $units, $lang, $appid, $mode, $url) |
|
428 | |||
429 | /** |
||
430 | * Builds the query string for the url. |
||
431 | * |
||
432 | * @param mixed $query |
||
433 | * |
||
434 | * @return string The built query string for the url. |
||
435 | * |
||
436 | * @throws \InvalidArgumentException If the query parameter is invalid. |
||
437 | */ |
||
438 | 7 | private function buildQueryUrlParameter($query) |
|
451 | |||
452 | /** |
||
453 | * @param string $answer The content returned by OpenWeatherMap. |
||
454 | * |
||
455 | * @return \SimpleXMLElement |
||
456 | * @throws OWMException If the content isn't valid XML. |
||
457 | */ |
||
458 | 7 | private function parseXML($answer) |
|
476 | } |
||
477 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.