Refactored #10
Refactored #10
Conversation
- updated examples - fixed linting errors - added namespace: SinusBot
src/API.class.php
Outdated
| @@ -398,7 +398,7 @@ public function getInstances() | |||
| $instances = $this->request('/bot/instances'); | |||
| $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"])); | |||
There was a problem hiding this comment.
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.
src/API.class.php
Outdated
| @@ -85,7 +85,7 @@ public function getPlaylists() | |||
| $playlists = $this->request('/bot/playlists'); | |||
| $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"])); | |||
There was a problem hiding this comment.
See comment on getInstances(), same thing applies here.
src/Instance.class.php
Outdated
| * Instance holds the Instance array | ||
| * @var array | ||
| */ | ||
| public $instance = null; |
There was a problem hiding this comment.
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.
src/Playlist.class.php
Outdated
| * Playlist holds the Playlist array | ||
| * @var array | ||
| */ | ||
| public $playlist = null; |
There was a problem hiding this comment.
Same thing applies here, although it's unclear what data is returned by the api since it doesn't appear to be documented.
mxschmitt
force-pushed
the
refactored
branch
from
b74705f to
1a42c34
Compare
src/Playlist.class.php
Outdated
| */ | ||
| public function getEntries() | ||
| { | ||
| return array_key_exists('entries', $this->playlist)?$this->playlist['entries']:''; |
There was a problem hiding this comment.
Default return should be [] instead of '', otherwise good 👍
Hi there,
in this PR I've refactored the whole library to be more flexible and modern. I've done:
Best regards
Max