Thanks for getting these. I still contend that it was highly immature of them to publicly criticize our code without first: 1) Giving us an opportunity to address their complaints 2) Fix any actual bugs
On Aug 4, 2009, at 1:33 AM, Michael C. Harris wrote: >> index.php (SuperGlobal) >> >> We think it is a very good idea to use objects instead of >> superglobals, >> because this decouples the application code from the global >> environment. >> This also makes it a lot easier to hand-craft input data, for example >> for unit tests. It's also a very good idea to not use $_REQUEST. >> But why >> overwrite the actual superglobals? >> >> Depending on whether process_gps() has already been executed or >> not, the >> superglobals will either contain regular arrays or objects, which we >> think is a bad design decision. You should not change the type of a >> variable halfway through the program flow. For one thing, the SuperGlobals are initialized in index.php. That's hardly "halfway through the program flow." >> >> We have seen code that explicitly unsets the superglobals to make it >> really clear to programmers that they are not supposed to use them. >> >> >> superglobal.php >> >> We think that reverting magic quotes should not happen in the same >> method than creating superglobal objects, because it's really a >> different concern. >> >> There is no way of telling that "work has already been done". Why not >> use a return value to indicate that? Or better, throw an exception if >> work has already been done, and thus force programmers to make sure >> they >> only call the method once. >> >> Why not consistently wrap all superglobals into objects and also >> convert >> $_COOKIE, $_ENV, and $_FILES? This seems like a good suggestion. >> >> What is the benefit of converting superglobals to objects if these >> objects implement ArrayIterators? You can use either like an array, >> which in turn means that you never know which you are using. That >> means >> at any point in the application, you can't tell whether you work with >> escaped or unescaped data. Additionally, is is very easy to tell if SuperGlobals have been initiated. Simply check for them being an object. >> >> The static variable $revert could be implemented as a static class >> member. Seems reasonable. >> >> >> session.php (destroy) >> >> The session class is tightly coupled to a database. It would be >> better >> design to have the database access in a separate class. Also seems reasonable. >> session.php (remove_error) >> >> Most coding standards require method names to be in camel case. Ours does not. In fact, it calls for the opposite. That's all that matters: our code is internally consistent. >> There is no reason to check for the unset result, it will always >> work. Nothing "always works" and there's no harm in checking. Some good suggestions, mixed in with some subjective drivel. --~--~---------~--~----~------------~-------~--~----~ To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/habari-dev -~----------~----~----~----~------~----~------~--~---
