[PEPr] Comment on XML::Create_KML

First off, there are a lot of reasons why you should never race to a 1.0. The most important is that alpha and beta versions allow you to adjust and improve the API, while this is nearly impossible once stable. Then on to a couple comments on the code: 1) First off, one-class-per-file rule. This means that all your classes should probably be called KML_*. Or if it's acceptable, XML_KML_*, e.g. XML_KML_Place, vs. KMLPlace, this results in a path name, XML/KML/Place.php. 2) It would be much nicer if you use used xmlwriter or simplexml to create your xml. 3) Unit tests would be very much appreciated, and I think they shouldn't be too hard to come up with since the result seems relatively straight forward. 4) You don't want to send headers in PHP code (because of among all the things why this is not a good idea, making it harder to unit test your code is one of the most obvious). 5) KMLStyle (or well, XML_KML_Style, or KML_Style), is not a bad idea, but I'd appreciate set*() and get*() that also validate input when needed, when needed. I'd suggest you implement those in KMLPlace, etc. as well. 6) For addItem(), you could force e.g. XML_KML_Common in the signature: function addItem(XML_KML_Common $item), then all the different objects extend from XML_KML_Common and you created a simple object hirachey. Could also make sense to add addStyle() (or maybe setStyle()) for the different types of objects. 7) In some cases exceptions wouldn't hurt -- e.g. for invalid arguments. FALSE is too plain, and since the methods can return FALSE for various reasons, it becomes tedious to debug. 8) With a lot of add*()/set*(), a fluent interface would be nice. 9) create() -- personally, I'd implement __toString() which would replace create() completely (or act as a wrapper). 10) Not sure if save() is needed. 11) I can see cloning come in handy -- so I suggest you implement the magic method. 12) Maybe a destructor to clean up? 13) Please implement empty constructors. 14) Last but not least, run PHP_CodeSniffer to fix all the obvious issues and to confirm to our coding standard. Anyway, as David said, he's setting up a sandbox on Github. You could commit your code and work on it and a couple of us could help you improve this so you can propose this code to PEAR. Let me know if you have any Qs! Till -- http://pear.php.net/pepr/pepr-proposal-show.php?id=616