Vote Details for "Net_GPS" by gauthierm

» Details
  • Voter: Michael Gauthier 
  • Vote: +1 (conditional)
  • Reviews: Cursory source review
» Comment
Cool package. I didn't even know mobile devices supported this behavior. Some comments:

* When documenting member variables, '@var object' should be '@var Net_UserAgent_Mobile'
* The method getGPSLink() could use more documentation. From what I can tell, this is how you display a link that would send GPS information from the phone when clicked.
* I'd like to see the interface Net_GPS_ProviderInterface in a separate file.
* Document thrown exceptions with @throws tag.
* check class_exists() before instantiating a driver.
* make sure class name doesn't contain directories ('../FooBar' for example) before including driver file.
* Perhaps an abstract class rather than an interface would be appropriate for Net_GPS_Provider. The Constructor seems to be copied four times.
* I agree with David about the package name.