Lastfm - Code Metrics - Inspection of "Massive refactorings" - phanan/koel - Measure and Improve Code Quality continuously with Scrutinizer

Test Setup Failed

Push — master ( 0a8e7c...aa7267 )

by Phan

created 2017-06-03 23:22 UTC

Lastfm B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	343
Duplicated Lines	14.29 %

Coupling/Cohesion

Components	1
Dependencies	2

Importance

Changes

Metric	Value
wmc	43
lcom	1
cbo	2
dl	49
loc	343
rs	8.3157
c	0
b	0
f	0

13 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	9	4
A	used()	0	4	2
A	enabled()	0	4	2
C	getArtistInfo()	7	32	7
A	buildArtistInfo()	0	11	2
C	getAlbumInfo()	7	33	7
A	buildAlbumInfo()	0	18	2
A	getSessionKey()	0	15	2
A	scrobble()	18	18	3
A	toggleLoveTrack()	0	13	3
A	updateNowPlaying()	17	17	3
B	buildAuthCallParams()	0	25	4
A	formatText()	0	8	2

How to fix Duplicated Code Complexity

Duplicated Code

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:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like Lastfm 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 Lastfm, and based on these observations, apply Extract Interface, too.

<?php

namespace App\Services;

use Exception;
use GuzzleHttp\Client;
use Illuminate\Support\Facades\Cache;
use Log;

class Lastfm extends RESTfulService
{
    /**
     * Specify the response format, since Last.fm only returns XML.
     *
     * @var string
     */
    protected $responseFormat = 'xml';

    /**
     * Override the key param, since, again, Lastfm wants to be different.
     *
     * @var string
     */
    protected $keyParam = 'api_key';

    /**
     * Construct an instance of Lastfm service.
     *
     * @param string $key    Last.fm API key.
     * @param string $secret Last.fm API shared secret.
     * @param Client $client The Guzzle HTTP client.
     */
    public function __construct($key = null, $secret = null, Client $client = null)
    {
        parent::__construct(
            $key ?: config('koel.lastfm.key'),
            $secret ?: config('koel.lastfm.secret'),
            'https://ws.audioscrobbler.com/2.0',
            $client ?: new Client()
        );
    }

    /**
     * Determine if our application is using Last.fm.
     *
     * @return bool
     */
    public function used()
    {
        return config('koel.lastfm.key') && config('koel.lastfm.secret');
    }

    /**
     * Determine if Last.fm integration is enabled.
     *
     * @return bool
     */
    public function enabled()
    {
        return $this->getKey() && $this->getSecret();
    }

    /**
     * Get information about an artist.
     *
     * @param $name string Name of the artist
     *
     * @return array|false
     */
    public function getArtistInfo($name)
    {
        if (!$this->enabled()) {
            return false;
        }

        $name = urlencode($name);

        try {
            $cacheKey = md5("lastfm_artist_$name");

            if ($response = cache($cacheKey)) {

                $response = simplexml_load_string($response);
            } else {
                if ($response = $this->get("?method=artist.getInfo&autocorrect=1&artist=$name")) {
                    cache([$cacheKey => $response->asXML()], 24 * 60 * 7);
                }
            }

            $response = json_decode(json_encode($response), true);

            if (!$response || !$artist = array_get($response, 'artist')) {
                return false;
            }

            return $this->buildArtistInfo($artist);
        } catch (Exception $e) {
            Log::error($e);

            return false;
        }
    }

    /**
     * Build a Koel-usable array of artist information using the data from Last.fm.
     *
     * @param array $lastfmArtist
     *
     * @return array
     */
    private function buildArtistInfo(array $lastfmArtist)
    {
        return [
            'url' => array_get($lastfmArtist, 'url'),
            'image' => count($lastfmArtist['image']) > 3 ? $lastfmArtist['image'][3] : $lastfmArtist['image'][0],
            'bio' => [
                'summary' => $this->formatText(array_get($lastfmArtist, 'bio.summary')),
                'full' => $this->formatText(array_get($lastfmArtist, 'bio.content')),
            ],
        ];
    }

    /**
     * Get information about an album.
     *
     * @param string $name       Name of the album
     * @param string $artistName Name of the artist
     *
     * @return array|false
     */
    public function getAlbumInfo($name, $artistName)
    {
        if (!$this->enabled()) {
            return false;
        }

        $name = urlencode($name);
        $artistName = urlencode($artistName);

        try {
            $cacheKey = md5("lastfm_album_{$name}_{$artistName}");

            if ($response = cache($cacheKey)) {

                $response = simplexml_load_string($response);
            } else {
                if ($response = $this->get("?method=album.getInfo&autocorrect=1&album=$name&artist=$artistName")) {
                    cache([$cacheKey => $response->asXML()], 24 * 60 * 7);
                }
            }

            $response = json_decode(json_encode($response), true);

            if (!$response || !$album = array_get($response, 'album')) {
                return false;
            }

            return $this->buildAlbumInfo($album);
        } catch (Exception $e) {
            Log::error($e);

            return false;
        }
    }

    /**
     * Build a Koel-usable array of album information using the data from Last.fm.
     *
     * @param array $lastfmAlbum
     *
     * @return array
     */
    private function buildAlbumInfo(array $lastfmAlbum)
    {
        return [
            'url' => array_get($lastfmAlbum, 'url'),
            'image' => count($lastfmAlbum['image']) > 3 ? $lastfmAlbum['image'][3] : $lastfmAlbum['image'][0],
            'wiki' => [
                'summary' => $this->formatText(array_get($lastfmAlbum, 'wiki.summary')),
                'full' => $this->formatText(array_get($lastfmAlbum, 'wiki.content')),
            ],
            'tracks' => array_map(function ($track) {
                return [
                    'title' => $track['name'],
                    'length' => (int) $track['duration'],
                    'url' => $track['url'],
                ];
            }, array_get($lastfmAlbum, 'tracks.track', [])),
        ];
    }

    /**
     * Get Last.fm's session key for the authenticated user using a token.
     *
     * @param string $token The token after successfully connecting to Last.fm
     *
     * @link http://www.last.fm/api/webauth#4
     *
     * @return string The token key
     */
    public function getSessionKey($token)
    {
        $query = $this->buildAuthCallParams([
            'method' => 'auth.getSession',
            'token' => $token,
        ], true);

        try {
            return (string) $this->get("/?$query", [], false)->session->key;

        } catch (Exception $e) {
            Log::error($e);

            return false;
        }
    }

    /**
     * Scrobble a song.
     *
     * @param string     $artist    The artist name
     * @param string     $track     The track name
     * @param string|int $timestamp The UNIX timestamp
     * @param string     $album     The album name
     * @param string     $sk        The session key
     *
     * @return bool
     */
    public function scrobble($artist, $track, $timestamp, $album, $sk)

    {
        $params = compact('artist', 'track', 'timestamp', 'sk');

        if ($album) {
            $params['album'] = $album;
        }

        $params['method'] = 'track.scrobble';

        try {
            return (bool) $this->post('/', $this->buildAuthCallParams($params), false);

        } catch (Exception $e) {
            Log::error($e);

            return false;
        }
    }

    /**
     * Love or unlove a track on Last.fm.
     *
     * @param string $track  The track name
     * @param string $artist The artist's name
     * @param string $sk     The session key
     * @param bool   $love   Whether to love or unlove. Such cheesy terms... urrgggh
     *
     * @return bool
     */
    public function toggleLoveTrack($track, $artist, $sk, $love = true)
    {
        $params = compact('track', 'artist', 'sk');
        $params['method'] = $love ? 'track.love' : 'track.unlove';

        try {
            return (bool) $this->post('/', $this->buildAuthCallParams($params), false);

        } catch (Exception $e) {
            Log::error($e);

            return false;
        }
    }

    /**
     * Update a track's "now playing" on Last.fm.
     *
     * @param string    $artist   Name of the artist
     * @param string    $track    Name of the track
     * @param string    $album    Name of the album
     * @param int|float $duration Duration of the track, in seconds
     * @param string    $sk       The session key
     *
     * @return bool
     */
    public function updateNowPlaying($artist, $track, $album, $duration, $sk)

    {
        $params = compact('artist', 'track', 'duration', 'sk');
        $params['method'] = 'track.updateNowPlaying';

        if ($album) {
            $params['album'] = $album;
        }

        try {
            return (bool) $this->post('/', $this->buildAuthCallParams($params), false);

        } catch (Exception $e) {
            Log::error($e);

            return false;
        }
    }

    /**
     * Build the parameters to use for _authenticated_ Last.fm API calls.
     * Such calls require:
     * - The API key (api_key)
     * - The API signature (api_sig).
     *
     * @link http://www.last.fm/api/webauth#5
     *
     * @param array $params   The array of parameters.
     * @param bool  $toString Whether to turn the array into a query string
     *
     * @return array|string
     */
    public function buildAuthCallParams(array $params, $toString = false)
    {
        $params['api_key'] = $this->getKey();
        ksort($params);

        // Generate the API signature.
        // @link http://www.last.fm/api/webauth#6
        $str = '';
        foreach ($params as $name => $value) {
            $str .= $name.$value;
        }
        $str .= $this->getSecret();
        $params['api_sig'] = md5($str);

        if (!$toString) {
            return $params;
        }

        $query = '';
        foreach ($params as $key => $value) {
            $query .= "$key=$value&";
        }

        return rtrim($query, '&');
    }

    /**
     * Correctly format a string returned by Last.fm.
     *
     * @param string $str
     *
     * @return string
     */
    protected function formatText($str)
    {
        if (!$str) {
            return '';
        }

        return trim(str_replace('Read more on Last.fm', '', nl2br(strip_tags(html_entity_decode($str)))));
    }
}


1		<?php
2
3		namespace App\Services;
4
5		use Exception;
6		use GuzzleHttp\Client;
7		use Illuminate\Support\Facades\Cache;
8		use Log;
9
10		class Lastfm extends RESTfulService
11		{
12		/**
13		* Specify the response format, since Last.fm only returns XML.
14		*
15		* @var string
16		*/
17		protected $responseFormat = 'xml';
18
19		/**
20		* Override the key param, since, again, Lastfm wants to be different.
21		*
22		* @var string
23		*/
24		protected $keyParam = 'api_key';
25
26		/**
27		* Construct an instance of Lastfm service.
28		*
29		* @param string $key Last.fm API key.
30		* @param string $secret Last.fm API shared secret.
31		* @param Client $client The Guzzle HTTP client.
32		*/
33		public function __construct($key = null, $secret = null, Client $client = null)
34		{
35		parent::__construct(
36		$key ?: config('koel.lastfm.key'),
37		$secret ?: config('koel.lastfm.secret'),
38		'https://ws.audioscrobbler.com/2.0',
39		$client ?: new Client()
40		);
41		}
42
43		/**
44		* Determine if our application is using Last.fm.
45		*
46		* @return bool
47		*/
48		public function used()
49		{
50		return config('koel.lastfm.key') && config('koel.lastfm.secret');
51		}
52
53		/**
54		* Determine if Last.fm integration is enabled.
55		*
56		* @return bool
57		*/
58		public function enabled()
59		{
60		return $this->getKey() && $this->getSecret();
61		}
62
63		/**
64		* Get information about an artist.
65		*
66		* @param $name string Name of the artist
67		*
68		* @return array\|false
69		*/
70		public function getArtistInfo($name)
71		{
72		if (!$this->enabled()) {
73		return false;
74		}
75
76		$name = urlencode($name);
77
78		try {
79		$cacheKey = md5("lastfm_artist_$name");
80
81	View Code Duplication	if ($response = cache($cacheKey)) {
		0 ignored issues – show Duplication introduced 2017-06-03 00:27 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
82		$response = simplexml_load_string($response);
83		} else {
84		if ($response = $this->get("?method=artist.getInfo&autocorrect=1&artist=$name")) {
85		cache([$cacheKey => $response->asXML()], 24 * 60 * 7);
86		}
87		}
88
89		$response = json_decode(json_encode($response), true);
90
91		if (!$response \|\| !$artist = array_get($response, 'artist')) {
92		return false;
93		}
94
95		return $this->buildArtistInfo($artist);
96		} catch (Exception $e) {
97		Log::error($e);
98
99		return false;
100		}
101		}
102
103		/**
104		* Build a Koel-usable array of artist information using the data from Last.fm.
105		*
106		* @param array $lastfmArtist
107		*
108		* @return array
109		*/
110		private function buildArtistInfo(array $lastfmArtist)
111		{
112		return [
113		'url' => array_get($lastfmArtist, 'url'),
114		'image' => count($lastfmArtist['image']) > 3 ? $lastfmArtist['image'][3] : $lastfmArtist['image'][0],
115		'bio' => [
116		'summary' => $this->formatText(array_get($lastfmArtist, 'bio.summary')),
117		'full' => $this->formatText(array_get($lastfmArtist, 'bio.content')),
118		],
119		];
120		}
121
122		/**
123		* Get information about an album.
124		*
125		* @param string $name Name of the album
126		* @param string $artistName Name of the artist
127		*
128		* @return array\|false
129		*/
130		public function getAlbumInfo($name, $artistName)
131		{
132		if (!$this->enabled()) {
133		return false;
134		}
135
136		$name = urlencode($name);
137		$artistName = urlencode($artistName);
138
139		try {
140		$cacheKey = md5("lastfm_album_{$name}_{$artistName}");
141
142	View Code Duplication	if ($response = cache($cacheKey)) {
		0 ignored issues – show Duplication introduced 2017-06-03 00:27 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
143		$response = simplexml_load_string($response);
144		} else {
145		if ($response = $this->get("?method=album.getInfo&autocorrect=1&album=$name&artist=$artistName")) {
146		cache([$cacheKey => $response->asXML()], 24 * 60 * 7);
147		}
148		}
149
150		$response = json_decode(json_encode($response), true);
151
152		if (!$response \|\| !$album = array_get($response, 'album')) {
153		return false;
154		}
155
156		return $this->buildAlbumInfo($album);
157		} catch (Exception $e) {
158		Log::error($e);
159
160		return false;
161		}
162		}
163
164		/**
165		* Build a Koel-usable array of album information using the data from Last.fm.
166		*
167		* @param array $lastfmAlbum
168		*
169		* @return array
170		*/
171		private function buildAlbumInfo(array $lastfmAlbum)
172		{
173		return [
174		'url' => array_get($lastfmAlbum, 'url'),
175		'image' => count($lastfmAlbum['image']) > 3 ? $lastfmAlbum['image'][3] : $lastfmAlbum['image'][0],
176		'wiki' => [
177		'summary' => $this->formatText(array_get($lastfmAlbum, 'wiki.summary')),
178		'full' => $this->formatText(array_get($lastfmAlbum, 'wiki.content')),
179		],
180		'tracks' => array_map(function ($track) {
181		return [
182		'title' => $track['name'],
183		'length' => (int) $track['duration'],
184		'url' => $track['url'],
185		];
186		}, array_get($lastfmAlbum, 'tracks.track', [])),
187		];
188		}
189
190		/**
191		* Get Last.fm's session key for the authenticated user using a token.
192		*
193		* @param string $token The token after successfully connecting to Last.fm
194		*
195		* @link http://www.last.fm/api/webauth#4
196		*
197		* @return string The token key
198		*/
199		public function getSessionKey($token)
200		{
201		$query = $this->buildAuthCallParams([
202		'method' => 'auth.getSession',
203		'token' => $token,
204		], true);
205
206		try {
207		return (string) $this->get("/?$query", [], false)->session->key;
		0 ignored issues – show Unused Code introduced 2017-06-03 23:25 UTC by Report Bug Copy Issue Report The call to `Lastfm::get()` has too many arguments starting with `array()`. This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue. If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. In this case you can add the `@ignore` PhpDoc annotation to the duplicate definition and it will be ignored. Loading history...
208		} catch (Exception $e) {
209		Log::error($e);
210
211		return false;
212		}
213		}
214
215		/**
216		* Scrobble a song.
217		*
218		* @param string $artist The artist name
219		* @param string $track The track name
220		* @param string\|int $timestamp The UNIX timestamp
221		* @param string $album The album name
222		* @param string $sk The session key
223		*
224		* @return bool
225		*/
226	View Code Duplication	public function scrobble($artist, $track, $timestamp, $album, $sk)
		0 ignored issues – show Duplication introduced 2017-06-03 00:27 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
227		{
228		$params = compact('artist', 'track', 'timestamp', 'sk');
229
230		if ($album) {
231		$params['album'] = $album;
232		}
233
234		$params['method'] = 'track.scrobble';
235
236		try {
237		return (bool) $this->post('/', $this->buildAuthCallParams($params), false);
		0 ignored issues – show Unused Code introduced 2017-06-03 00:27 UTC by Report Bug Copy Issue Report The call to `Lastfm::post()` has too many arguments starting with `false`. This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue. If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. In this case you can add the `@ignore` PhpDoc annotation to the duplicate definition and it will be ignored. Loading history...
238		} catch (Exception $e) {
239		Log::error($e);
240
241		return false;
242		}
243		}
244
245		/**
246		* Love or unlove a track on Last.fm.
247		*
248		* @param string $track The track name
249		* @param string $artist The artist's name
250		* @param string $sk The session key
251		* @param bool $love Whether to love or unlove. Such cheesy terms... urrgggh
252		*
253		* @return bool
254		*/
255		public function toggleLoveTrack($track, $artist, $sk, $love = true)
256		{
257		$params = compact('track', 'artist', 'sk');
258		$params['method'] = $love ? 'track.love' : 'track.unlove';
259
260		try {
261		return (bool) $this->post('/', $this->buildAuthCallParams($params), false);
		0 ignored issues – show Unused Code introduced 2017-06-03 00:27 UTC by Report Bug Copy Issue Report The call to `Lastfm::post()` has too many arguments starting with `false`. This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue. If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. In this case you can add the `@ignore` PhpDoc annotation to the duplicate definition and it will be ignored. Loading history...
262		} catch (Exception $e) {
263		Log::error($e);
264
265		return false;
266		}
267		}
268
269		/**
270		* Update a track's "now playing" on Last.fm.
271		*
272		* @param string $artist Name of the artist
273		* @param string $track Name of the track
274		* @param string $album Name of the album
275		* @param int\|float $duration Duration of the track, in seconds
276		* @param string $sk The session key
277		*
278		* @return bool
279		*/
280	View Code Duplication	public function updateNowPlaying($artist, $track, $album, $duration, $sk)
		0 ignored issues – show Duplication introduced 2017-06-03 00:27 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
281		{
282		$params = compact('artist', 'track', 'duration', 'sk');
283		$params['method'] = 'track.updateNowPlaying';
284
285		if ($album) {
286		$params['album'] = $album;
287		}
288
289		try {
290		return (bool) $this->post('/', $this->buildAuthCallParams($params), false);
		0 ignored issues – show Unused Code introduced 2017-06-03 00:27 UTC by Report Bug Copy Issue Report The call to `Lastfm::post()` has too many arguments starting with `false`. This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue. If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress. In this case you can add the `@ignore` PhpDoc annotation to the duplicate definition and it will be ignored. Loading history...
291		} catch (Exception $e) {
292		Log::error($e);
293
294		return false;
295		}
296		}
297
298		/**
299		* Build the parameters to use for _authenticated_ Last.fm API calls.
300		* Such calls require:
301		* - The API key (api_key)
302		* - The API signature (api_sig).
303		*
304		* @link http://www.last.fm/api/webauth#5
305		*
306		* @param array $params The array of parameters.
307		* @param bool $toString Whether to turn the array into a query string
308		*
309		* @return array\|string
310		*/
311		public function buildAuthCallParams(array $params, $toString = false)
312		{
313		$params['api_key'] = $this->getKey();
314		ksort($params);
315
316		// Generate the API signature.
317		// @link http://www.last.fm/api/webauth#6
318		$str = '';
319		foreach ($params as $name => $value) {
320		$str .= $name.$value;
321		}
322		$str .= $this->getSecret();
323		$params['api_sig'] = md5($str);
324
325		if (!$toString) {
326		return $params;
327		}
328
329		$query = '';
330		foreach ($params as $key => $value) {
331		$query .= "$key=$value&";
332		}
333
334		return rtrim($query, '&');
335		}
336
337		/**
338		* Correctly format a string returned by Last.fm.
339		*
340		* @param string $str
341		*
342		* @return string
343		*/
344		protected function formatText($str)
345		{
346		if (!$str) {
347		return '';
348		}
349
350		return trim(str_replace('Read more on Last.fm', '', nl2br(strip_tags(html_entity_decode($str)))));
351		}
352		}
353

phanan / koel