[D7] LAKELET Qrigo

Hello, the automated pareview recognized these issues, please address:
http://ventral.org/pareview/httpgitdrupalorgsandboxlchang1991572git

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.

General
Something that's not precisely wrong but just makes your code difficult to read: Maybe add some new lines to seperate blocks of code. It feels similar to reading a text without any paragraphs.

Comments
A lot of functions(none hooks) miss commenting about the parameters and the return.

For example:

/**
 * Sets an unique filename for a barcode image.
 */
function lakelet_qrigo_set_barcode_image_filename($text, $settings) {
  $stamp = array(
    $text,
    $settings['lakelet_qrigo_error_correction_level'],
    $settings['lakelet_qrigo_module_size'],
    $settings['lakelet_qrigo_quiet_zone_width'],
  );
  if ($settings['lakelet_qrigo_enable_branding']) {
    $stamp[] = hash_file('sha256', drupal_realpath($settings['lakelet_qrigo_logo_file']));
  }
  return $settings['lakelet_qrigo_file_path'] . '/' . hash('sha256', serialize($stamp)) . '.png';
}

or

/**
 * Brands a barcode image.
 */
function lakelet_qrigo_stud_logo($barcode_image, $logo_image, $logo_resize_factor) {
  $barcode_width = $barcode_image->info['width'];
  $barcode_height = $barcode_image->info['height'];
  $barcode_image_bg = imagecreatetruecolor($barcode_width, $barcode_height);
  imagecopy($barcode_image_bg, $barcode_image->resource, 0, 0, 0, 0, $barcode_width, $barcode_height);
  $barcode_image->resource = $barcode_image_bg;
  $logo_width = $logo_image->info['width'];
  $logo_height = $logo_image->info['height'];
  $sqrt_logo_resize_factor = sqrt($logo_resize_factor / 100);
  $dst_w = $sqrt_logo_resize_factor * $barcode_width;
  $dst_h = $sqrt_logo_resize_factor * $barcode_height;
  $dst_x = $barcode_width / 2 - $dst_w / 2;
  $dst_y = $barcode_height / 2 - $dst_h / 2;
  imagecopyresampled($barcode_image->resource, $logo_image->resource, $dst_x, $dst_y, 0, 0, $dst_w, $dst_h, $logo_width, $logo_height);
  return $barcode_image;
}

See: http://drupal.org/node/1354#param

Please check all your functions to see if it misses any doxygen comments. For the whole list see http://drupal.org/node/1354

Hello,

You have a lot of codes in the .module,

You should separate the code of your settings page into a lakelet_qrigo.admin.inc,
you can specify the filepath in your menu definition, so you can put into another file, all your settings form functions.
you can do the same with the function that generate the barcode into a lakelet_qrigo.barcode.inc, and then use module_load_include() to load that files when it's needed.

Some function are missing inline comments and/or put some line separation into your code :
lakelet_qrigo_label_deliver_html_page()
lakelet_qrigo_field_formatter_view()
lakelet_qrigo_create_barcode_image()
lakelet_qrigo_create_barcode_image()
lakelet_qrigo_admin_settings_form() also avoid variable names like $ei, $et, prefer $entity_info, $entity_type, that's better for everyone.
lakelet_qrigo_common_settings_form()

Make your code readable, and hierarchised, make it easier to understand and maintain for you and for everyone.

also
.module, line 647

$length = count($parents);
$length--;

can be

$length = count($parents) - 1;

good luck

This sounds like a feature that should live in the existing qr_codes project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the qr_codes issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

Hi Lchang,

I reviewed your module, I just want to share my thoughts.

This module requires, PHP QR code library. It will help to others, if you shown the message when user enter to this module settings page.

Could you please add more info on readme.txt for better understanding.

Regards,
-Raj.

@lchang: you can already use your functions and others can too, since this is a public sandbox on drupal.org.

We have tons of QR code modules already on drupal.org and adding yet another one will not help anybody.

And since the qr_codes module is unmaintained anyway you should just take it over instead of creating something new. So please follow the abandoned module process first before we continue here: http://drupal.org/node/251466

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

Reviews of other projects: [D7] Cache_node_object