Musixmatch::searchLyricsRelatedToSong() - Code Metrics - Inspection of "Musixmatch service integration to populate each so..." - phanan/koel - Measure and Improve Code Quality continuously with Scrutinizer

Test Setup Failed

Pull Request — master (#623)

by Alejandro Carstens

created 2017-07-18 23:50 UTC

Musixmatch::searchLyricsRelatedToSong() A

↳ Parent: Musixmatch

Complexity

Conditions	4
Paths	3

Size

Total Lines	15
Code Lines	8

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	15
rs	9.2
c	0
b	0
f	0
cc	4
eloc	8
nc	3
nop	1

<?php

namespace App\Services;

use App\Models\Song;
use GuzzleHttp\Client;
use Log;

class Musixmatch extends RESTfulService
{
    /**
     * Construct an instance of Musixmatch service.
     *
     * @param string      $key    The Musixmatch API key
     * @param Client|null $client The Guzzle HTTP client
     */
    public function __construct($key = null, Client $client = null)

    {
        parent::__construct(
            $key ?: config('koel.musixmatch.key'),
            null,
            'https://api.musixmatch.com/ws/1.1',
            $client ?: new Client()
        );
    }
    
    /**
     * Determine if our application is using Musixmatch.
     *
     * @return bool
     */
    public function enabled()
    {
        return (bool) config('koel.musixmatch.key');
    }
    
    /**
     * @param App\Models\Song $song
     * 
     * @return mixed|null
     */
    public function searchLyricsRelatedToSong(Song $song)
    {
        if($song->hasLyrics() == false)

        {
            if (!$song->artist->isUnknown() && !$song->artist->isVarious()) {

                $lyrics = $this->search($song->title, $song->artist->name);
            } else {
                $lyrics = $this->search($song->title);
            }
            
            return $song->updateSingle($song->title, $song->album->name, $song->artist->name, $lyrics, $song->track, (int) $song->compilationState);
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        }
        
        return false;
    }
    
    /**
     * @param string $name
     * @param string $artistName
     * 
     * @return mixed|false
     */
    public function search(string $name, string $artistName = null)
    {
        if (!$this->enabled()) {
            return false;
        }
        
        $uri = sprintf('matcher.lyrics.get?format=jsonp&callback=callback&q_track=%s&q_artist=%s&apikey=%s',
            urlencode($name),
            urlencode($artistName),
            $this->key
        );
        
        try{
            $response = $this->jsonpDecode($this->get($uri));
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            
            return $this->getLyrics($response);
        }catch(\Exception $e){
            Log::error($e);

            return false;
        }
    }
    
    /**
     * @param string $response
     * 
     * @return mixed
     */
    private function jsonpDecode($response)
    {
        return json_decode(substr(preg_replace("/[^(]*\((.*)\)/","$1",$response), 0, -1));
    }
    
    /**
     * @param JSON $response
     * 
     * @return string|null
     */
    private function getLyrics($response)
    {
        $lyrics = $response->message->body->lyrics->lyrics_body;
        
        if(strlen($lyrics))
        {
            // Correct Musixmatch spelling mistake (is for are).
            return substr($lyrics, 0, strpos($lyrics, "******* This Lyrics is NOT for Commercial use *******")).
                "*** This Lyrics are NOT for Commercial use ***";
        }
    }
}

1		<?php
2
3		namespace App\Services;
4
5		use App\Models\Song;
6		use GuzzleHttp\Client;
7		use Log;
8
9		class Musixmatch extends RESTfulService
10		{
11		/**
12		* Construct an instance of Musixmatch service.
13		*
14		* @param string $key The Musixmatch API key
15		* @param Client\|null $client The Guzzle HTTP client
16		*/
17	View Code Duplication	public function __construct($key = null, Client $client = null)
		0 ignored issues – show Duplication introduced 2017-07-17 23:56 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...
18		{
19		parent::__construct(
20		$key ?: config('koel.musixmatch.key'),
21		null,
22		'https://api.musixmatch.com/ws/1.1',
23		$client ?: new Client()
24		);
25		}
26
27		/**
28		* Determine if our application is using Musixmatch.
29		*
30		* @return bool
31		*/
32		public function enabled()
33		{
34		return (bool) config('koel.musixmatch.key');
35		}
36
37		/**
38		* @param App\Models\Song $song
39		*
40		* @return mixed\|null
41		*/
42		public function searchLyricsRelatedToSong(Song $song)
43		{
44		if($song->hasLyrics() == false)
		0 ignored issues – show Coding Style Best Practice introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report It seems like you are loosely comparing two booleans. Considering using the strict comparison `===` instead. When comparing two booleans, it is generally considered safer to use the strict comparison operator. Loading history...
45		{
46		if (!$song->artist->isUnknown() && !$song->artist->isVarious()) {
		0 ignored issues – show Bug introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report The method `isUnknown()` does not exist on `App\Models\Artist`. Did you maybe mean `getIsUnknownAttribute()`? This check marks calls to methods that do not seem to exist on an object. This is most likely the result of a method being renamed without all references to it being renamed likewise. Loading history... Bug introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report The method `isVarious()` does not exist on `App\Models\Artist`. Did you maybe mean `getIsVariousAttribute()`? This check marks calls to methods that do not seem to exist on an object. This is most likely the result of a method being renamed without all references to it being renamed likewise. Loading history...
47		$lyrics = $this->search($song->title, $song->artist->name);
48		} else {
49		$lyrics = $this->search($song->title);
50		}
51
52		return $song->updateSingle($song->title, $song->album->name, $song->artist->name, $lyrics, $song->track, (int) $song->compilationState);
		0 ignored issues – show Documentation introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report The property `compilationState` does not exist on `object<App\Models\Song>`. Since you implemented `__get`, maybe consider adding a @property annotation. Since your code implements the magic getter `_get`, this function will be called for any read access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text / class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } If the property has read access only, you can use the @property-read annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history... Bug introduced 2017-07-18 23:54 UTC by Report Bug Copy Issue Report It seems like `$lyrics` can also be of type `false` or `null`; however, `App\Models\Song::updateSingle()` does only seem to accept `string`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /* * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
53		}
54
55		return false;
56		}
57
58		/**
59		* @param string $name
60		* @param string $artistName
61		*
62		* @return mixed\|false
63		*/
64		public function search(string $name, string $artistName = null)
65		{
66		if (!$this->enabled()) {
67		return false;
68		}
69
70		$uri = sprintf('matcher.lyrics.get?format=jsonp&callback=callback&q_track=%s&q_artist=%s&apikey=%s',
71		urlencode($name),
72		urlencode($artistName),
73		$this->key
74		);
75
76		try{
77		$response = $this->jsonpDecode($this->get($uri));
		0 ignored issues – show Documentation introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report `$this->get($uri)` is of type `object`, but the function expects a `string`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
78
79		return $this->getLyrics($response);
80		}catch(\Exception $e){
81		Log::error($e);
82
83		return false;
84		}
85		}
86
87		/**
88		* @param string $response
89		*
90		* @return mixed
91		*/
92		private function jsonpDecode($response)
93		{
94		return json_decode(substr(preg_replace("/[^(]\((.)\)/","$1",$response), 0, -1));
95		}
96
97		/**
98		* @param JSON $response
99		*
100		* @return string\|null
101		*/
102		private function getLyrics($response)
103		{
104		$lyrics = $response->message->body->lyrics->lyrics_body;
105
106		if(strlen($lyrics))
107		{
108		// Correct Musixmatch spelling mistake (is for are).
109		return substr($lyrics, 0, strpos($lyrics, "***** This Lyrics is NOT for Commercial use *****")).
110		"* This Lyrics are NOT for Commercial use *";
111		}
112		}
113		}

phanan / koel

Pull Request — master (#623)

Musixmatch::searchLyricsRelatedToSong() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like