[PEPr] +1 for Networking::Net_MPD
| From: | Christian Weiske | Date: | Wed, 02 Aug 2006 13:56:33 +0000 |
| Subject: | [PEPr] +1 for Networking::Net_MPD | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-43619@lists.php.net to get a copy of this message | ||
Christian Weiske (http://pear.php.net/user/cweiske) has voted +1 on the proposal for
Networking::Net_MPD.
Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=429
Vote information:
http://pear.php.net/pepr/pepr-vote-show.php?id=429&handle=cweiske
Comment:
I vote +1 despite some problems:
- Please conform to the PEAR CS, especially regarding brackets in function
and class declarations, as well as indentation
- the example should be delivered with the package as "doc" role
- the example does "require_once 'MPD.php';" although it should be
Net/MPD.php
- An example that lists all songs in the current playlist would be nice,
and should be delivered with the package
- Net_MPD_Playlist::add doesn't work if there are quotes in the filename
- Return values are not very optimal; I saw it in "currentSong()":
array(1) {
["file"]=>
array(1) {
[1]=>
array(4) {
["file"]=>
string(54) "somefile.ogg"
["Time"]=>
string(3) "212"
["Pos"]=>
string(2) "91"
["Id"]=>
string(4) "1287"
}
}
}
I mean it is *one* current song, isn't it? Why do I need an array with a
"file" in it, and why do I need a subarray when only one song is
expected?
I think many of the other methods have similar "standard" return values
that are too complicated mostly. (playlistInfo() does also return an array
with "file" key and the files as array below it - why is the outer array
needed?
- please use "get" as prefix for methods that return something, and "set"
for setter methods.
- return value descriptions are often wrong, e.g. in playlist/swapSong:
the docblock says a boolean value is returned, but that's only true if all
is ok - an interger can be returned as well.
- There is no method to list all playlists in the playlist class
- the "ID" in method names should be "Id"
--
Sent by PEPr, the automatic proposal system at http://pear.php.net