Alexander Wirtz [2004-03-16 10:23 UTC] Why do you propose this into a new main structure "Games"? Don't you think it'd be more appropriate to place it into Networking or even Web_Services? IMHO you should've asked on pear-dev first, if the package name is acceptable ;-)
Aidan Lister [2004-03-16 10:33 UTC] I chose this name based on advice from Pierre and Stefan Neufeind.
I think Net_GameQueryServer is just as good, although a bit long.
Even Net_QueryServer, or Net_GameQuery would be fine by myself.
You'll note reading back through pear-dev I actually asked for more information regarding a suitable name.
David Grant [2004-03-16 10:39 UTC] This is certainly very interesting, Aidan, and something I might use myself at a later date. I would request that you follow the coding standards more tightly, however.
Klaus Guenther [2004-03-16 10:47 UTC] Alex: He didn't propose anything new. We already have a package using the name category Games_:
http://pear.php.net/package/Games_Chess
However, it is listed under structures.
Now for some comments of my own:
You have consistantly misspelled receive as "recieve" (see _recieve() )
The properties still need documentation, even if they are private.
As David commented, you'll need to modify to follow coding standards (e.g., if statements need braces).
Aidan Lister [2004-03-16 11:55 UTC] I have made all the suggested changes.
Aidan Lister [2004-03-17 10:20 UTC] I believe I've covered all the expressed concerns, if this is incorrect please let me know!
David Grant [2004-03-17 11:08 UTC] You've commented throughout your source code of your intention to throw errors instead of exiting the script. I would advise you to implement this before the next proposal.
Stefan Neufeind [2004-03-17 11:14 UTC] Looks nice from what I saw. Had a look at examples and code.
You're using php5-syntax. Though this is generally good I personally would favor to be able to run it with php4.x for a while longer, also. I mean, the private-declarations could maybe be avoided and noted in the phpdoc-comments? Haven't tried, but if there is nothing else that won't work with 4.x imho it might be good to stay backward-compatible.
Other packages which have decided to move on to an php5-only already are afaik always having a still existant 4.x-branch.
Could it be that the "Day of Defeat"-example does not always work? Several times I got "Query took 1000.89ms" or similar instead of the answer. Maybe timing-problems?
And please do some fixing to the phpdoc-comments. E.g. for variables you need @var, the whole package should also have a doc-blook, include license at the top, I'd prefer to see the author listed once - not for every function and similar.
The package is still quite small. Does this result from the fact that all servers react quite similar? Maybe more games could be added? And maybe you could try to provide some commonly named and format, parsed information fields for all game-servers as well.
Aidan Lister [2004-03-18 00:48 UTC] David Grant,
I'll implement this right away - I wasn't sure what to do with all this Error_Stack talk.
Stefan Neufeind,
Yes, It's done using php5 syntax - there are no functions that rely on php5 being installed, so I'll create a php4 version as well.
I belive the way I've coded the package resulted in its small size - most other packages I've looked at that provided the same support used multiple and very long 'case' statements. I feel the package will grow exponentially once I start normalising the data and adding more specific features in - but hell,why not!
The DOD server may have been down at the time you tried it, resulting in a 1000ms timeout. This will (now) result in a thrown error.
Thank you for your in-depth review.
David Costa [2004-03-25 00:57 UTC] Looks good to me and it works under PHP 5. Well commented code too.
|
