Vote Details for "Net_IMInfo" by aidan
| » Details |
|---|
|
| » Comment |
|
I'm still not happy with the package, I think it's a good idea but in my mind you have not resolved all the problems. Linking to images stored anywhere other than inside the package is A Bad Thing. I personally do not see the need for images at all: each query should return one of the result constants (more about this later). Output can be decided by the user, it should not be handled by the package. You have not implemented plug-in protocols - I think this is a must. Ie. Do not hardcode it. The API should be something like: $this->getstatus('id', NET_IMINFO_ICQ); The result should be any of NET_IMINFO_RESULT_ONLINE/DISABLED/OFFLINE You have defined your CONSTANTS with @var string, this is wrong. But it's your use of constants which I don't like, for example when you define the sites you're contacting: I would do it as folows (I've removed the namespace) $sites = array( PROTO_ICQ => "http://...", PROTO_MSN => "http://.. etc. Constants should serve as an index, rather than a means of storing data throughout the script. I'm really sorry to give you a -1 vote, I love the idea of this package, however I just don't feel it's up to scratch. |