[D7] AddressField from Geofield

Dear renaudcuny,

Please use this online tool (pareview.sh) to check for general errors in your module code.

Checking your code using this tool shows lots of errors.
http://pareview.sh/pareview/httpgitdrupalorgsandboxrcuny2160073git

Please take your time to correct those issues.

Cheers!!!

Hi,

After reviewing some of your code, I have a couple of suggestions to improve your code.

Beginning at line 121:

  $info = "";
  foreach ($address->results as $address_info) {
    $info = _addressfield_from_geofield_extract_address_info_value($address_info, $request);
    if ($info != "") {
      break;
    }
  }

The if() statement should be replaced with this:

if (!empty($info)) {
    break;
}

There are several more instances of this. It is better practice to check for empty strings with empty(), rather than comparing empty strings.

Other than these small items, there are no noticeable errors with your code. Keep up the good work.

Cheers,
Adam

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hey,

Make a new branch called as 7.x and push the code there. Also don't forget to remove the master branch.

Hi @renaudcuny,

I reviewed you code, I found a using file_get_contents().
It's better to using drupal_http_request() instead of file_get_contents() to work with servers behind proxy.

renaudcuny there are few coding standards issue reported by pareview, can you address them http://pareview.sh/pareview/httpgitdrupalorgsandboxrcuny2160073git

@renaudcuny those where simple suggestion, not an application blocker

Project applications with Review bonus takes priority. So please complete manual reviews & include those url's here. Once done, please include appropriate tags in the application.

Please remove .gitignore.

Use https://api.drupal.org/api/drupal/modules%21system%21system.api.php/func... rather than adjusting module weight.

function addressfield_from_geofield_field_attach_presave($entity_type, $entity) {
      if ($geofield_value) {
        // Hardcoded, should move to field settings. Modify according to your needs.
        $force_language = "fr";
        addressfield_from_geofield_set_address_from_geodata($entity, $field_name, $geofield_value, $force_language);
      }

Hard coded language of FR seems an issue. Maybe declare global $language and pull language from there.

global $language;
$language_code = $language->language;

      // if you want to erase the address data when geocoding fails, uncomment the following:
      /*
      else {
      $entity->{$field_name} = array();
      }
      */

Either make this a configurable option (somehow) or remove it. Its not a best practice to hack core or contrib modules.

function addressfield_from_geofield_set_address_from_geodata($entity, $field_name, $geofield_value, $lang = NULL) {
...
  if ($address) {
    // Extract the interesting pieces of information from the address data.
    $street_number = _addressfield_from_geofield_extract_address_info($address, "street_number");
    $route         = _addressfield_from_geofield_extract_address_info($address, "route");
    $postal_code   = _addressfield_from_geofield_extract_address_info($address, "postal_code");
    $locality      = _addressfield_from_geofield_extract_address_info($address, "locality");
    $country       = _addressfield_from_geofield_extract_address_info($address, "country");

Use single spaces around the equals. https://drupal.org/coding-standards#operators

function addressfield_from_geofield_set_address_from_geodata($entity, $field_name, $geofield_value, $lang = NULL) {
...
    // To store something in "Address 2" field, here it goes
    // $entity->{$field_name}[LANGUAGE_NONE][0]['premise'] = "address 2 field";

Comments like this should either be addressed or noted as a @todo.

function _addressfield_from_geofield_reverse_geocode($lat, $lon, $lang = NULL) {
  $json = NULL;
  $language_setting = ($lang) ? '&language=' . $lang : '';
  $response = drupal_http_request('https://maps.googleapis.com/maps/api/geocode/json?latlng=' . $lat . ',' . $lon . '&sensor=false' . $language_setting);
  if (!isset($response->error)) {
    $json = json_decode($response->data);
  }
  return $json;

Maybe check for HTTP 200 status code too?

Also all comments, including inline comments should be under 80 characters. See bullet #8: https://drupal.org/coding-standards/docs#drupal

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.