current community

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

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
2  
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
add comment

closed as off-topic by codesparkle♦ Aug 31 '13 at 1:37

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must include the code you want reviewed. Code must not appear only in a link to an external source. Doing so makes us dependent on a third party and makes it harder to review your code. If your code is very large, please select only the portions in which you are especially interested in a review. You are encouraged to keep the link to the rest of your code." – codesparkle
If this question can be reworded to fit the rules in the help center, please edit the question.

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
add comment

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