Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Not really much else to say besides what's on the tin.

The project was only recently launched and has yet to be promoted. Wanted some colleague eyeballs on this before pulling that trigger.

https://github.com/JWT-OSS/Google-Geocoding-PHP-SDK-Unofficial

share|improve this question
the code is beautiful :) – RobertPitt Jan 27 '11 at 22:44
Consider adding some of the code in your question, as code in your repository will change over time. Since your code is great, adding some snippets here will also be very helpful as a reference to future visitors. – Yannis Nov 14 '11 at 6:56

1 Answer

up vote 1 down vote accepted

WOW

The code is organised, clean, dependable and great OOP, there is not much that you need to change but I do have a few suggestions that you may / may not be interested in but every little helps.

The first minor issue is that as your using the users clients scope to set a variable called $here, which may cause some issues if the user has already set a variable called $here (Minor Issue).

The "feature" that I would like to see is a namespace, encapsulating all your code within its own specific scope, this would take care of little issues like the $here variable.

If you do decide to create a version which is based around a namespace I would also recommend that you take a look at the php team's proposal over @Google Groups and the class which should be modified to suite your perfectionist needs (Located Here) - Large vendors such as Zend and Doctrine also use the same proposal.

Also the classes can be specifically name for there function without being prefixed with the vendor.

  • GoogleGeocodeException > Exception
  • GoogleGeocodeCommunicatorException > CommunicatorException

if you encapsulated within the namespace GoogleGeocode

otherwise I see how much effort and thought you have put into the code and its a credit to the PHP World, im sure that google would feature this as an External SDK.

Update:

After reading your source code a little more in depth and trying to understand the work flow I come across another small change and that is allowing array and objects to be passed into the GoogleGeocodeServiceV3::geocode.

Regards.

share|improve this answer
Great feedback! Yes indeed, we/I have plans to do a 5.3 fork with namespacing. Thanks for the link to the PSR. – Peter Bailey Jan 28 '11 at 0:11

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.