Skip to content

Conversation

mxschmitt
Copy link
Member

Hi there,

in this PR I've refactored the whole library to be more flexible and modern. I've done:

  • added Classes for Instance and Playlist. Track and Folder maybe follow.
  • added autoloader support
  • added phpdoc comments
  • added linting to be PSR2 compliant
  • removed example function return values

Best regards

Max

$out = [];
foreach ($instances as $instance) {
array_push($out, new Instance($this->token, $this->url, $this->timeout, $instance));
array_push($out, new Instance($this->token, $this->url, $this->timeout, $instance["uuid"]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By only passing the uuid you throw away the other information (i.e. nick, name, running) that could also be used without having to send another request for each instance. I'd prefer it if the entire instance data is passed.

$out = [];
foreach ($playlists as $playlist) {
array_push($out, new Playlist($this->token, $this->url, $this->timeout, $playlist));
array_push($out, new Playlist($this->token, $this->url, $this->timeout, $playlist["uuid"]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on getInstances(), same thing applies here.

* Instance holds the Instance array
* @var array
*/
public $instance = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this is probably a good idea, however it would be useful if you could still access the information that was returned by the api (i.e. nick, name, running).
I'm not sure what's the best solution for this. I would either save them in private variables and add getter methods or just save them in public variables like uuid.

* Playlist holds the Playlist array
* @var array
*/
public $playlist = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing applies here, although it's unclear what data is returned by the api since it doesn't appear to be documented.

*/
public function getEntries()
{
return array_key_exists('entries', $this->playlist)?$this->playlist['entries']:'';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default return should be [] instead of '', otherwise good 👍

@mxschmitt mxschmitt merged commit 4db8643 into master Sep 7, 2018
@mxschmitt mxschmitt deleted the refactored branch September 8, 2018 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants