chia [jasonleon]

Remember to change status, when you upload new code.

Please change only the status, when you upload new code.

See the Drupal coding standards to understand how a module code should be written.
In particular, see the part about the namespace respect, and the use of Drupal Unicode functions (which is then valid for all the functions Drupal makes available to third-party modules).

Also, I think that the ereg functions are not anymore present in PHP 5.3; the code should use the preg functions.

1.   if ($path == 'admin/help#sitestats'){
       $txt = 'This Module provide a tool that fetches various Statistics of a site such as pagerank, alexa rank, google backlinks, domain age';
       return '<p>'. t($txt) .'</p>';
     }
   

   The first argument of t() needs to be a literal string, not a variable; differently the script to extract the strings to translate will not find the string to translate.

2.     '#type' => 'textarea',
       '#title' => t("URL ($max_url max)"),
   

   The code should use a placeholder.

3.     '#description' => 'URLs separated by newline. Example: www.pagerankandalexa.com ',
   

   Every strings that are used in the user interface should be translatable.

4. function sitestats_getform_submit($form,&$form_state) {
     drupal_goto('sitestats', array('urls'=> $_REQUEST['url']));
   }
   

   A submission function doesn't use drupal_goto().

5.   require_once('func.php');
     $urls = explode("\r", $urls);
   
     $urls = array_map('trim', $urls);
     $urls = array_filter($urls, 'strlen');
     $urls = array_slice($urls, 0, variable_get('sitestats_max', 5));
   

   The statement require_once is looking for a file that is in the root directory of Drupal; the code should use module_load_include().
   Also, rather than using array_filter($urls, 'strlen'), the code could use array_filter($urls).

6.       $params['subject'] = t(variable_get('sitestats_mail_sub', SITESTATS_MAIL_SUB));
         $params['body'] = t(variable_get('sitestats_mail_body', SITESTATS_MAIL_BODY),$vars);
   

   This error has been already reported.

7.   $form['mail']['notified'] = array(
         '#value' => "Be notified when Pagerank on $url changes!",
         );
   

   The string is not translatable.

9.   if ( !db_result(db_query("SELECT * FROM {sitestats} WHERE hashelement='%s'",$r['hashelement'])))
       $return = drupal_write_record('sitestats', $r);
   

   IF-statements should use the curly brachets, as reported in the coding standards.

10.   $result['register'] = drupal_get_form('_sitestats_subscribe', $url);
      $length = variable_get('sitestats_url_length',25);
      if (drupal_strlen($url) > $length + 3) {
        $url = drupal_substr($url, 0, $length) .'...';
      }
    

    Why isn't the code using truncate_utf8()?

11.         watchdog('sitestats', "Mail Sent to $result->mail Updated pr from $oldpr to $newpr", NULL, WATCHDOG_INFO);
    

    It would be better to use placeholders.

12.   if (count($result) > 0) {
        drupal_set_message(t('Sitestats module Installed.'));
      }
      else {
        drupal_set_message(t('Sitestats table creation failed. Please "uninstall" the module and retry'));
      }
    

    Normally, the installation code just calls the function without to check the value returned.

13. if( variable_get('sitestats_alexa_backlink', TRUE) ) {
      $r[] = array( 'Alexa Backlink', $rows['alexa_backlink']!=''?$rows['alexa_backlink']:'Not Available');
    }
    

    See the Drupal coding standards to understand how a module code should be written.
    The coding standars also suggest to use Drupal Unicode functions, which handle better the multi-byte strings.

14.       '#title' => t('Email about the Change in the Pagerank'),
    

    Strings used like that should have the first word written in capital case, and the others written in lower case (with the exception of proper nouns, or acronyms).

15.     '#description' => t('Some Template may break if URL lenght is large, to prevent it set the url length and an ellipse will be added at the end(...)'),
    

    It should be 'Some templates may break if the URL length is too big.

16.   $form['sitestats_pr'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Google Pr?'),
        '#default_value' => variable_get('sitestats_pr', TRUE),
        );
      $form['sitestats_alexa_popularity'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Alexa Popularity?'),
        '#default_value' => variable_get('sitestats_alexa_popularity', TRUE),
        );
      $form['sitestats_alexa_backlink'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Alexa Backlink?'),
        '#default_value' => variable_get('sitestats_alexa_backlink', TRUE),
        );
      $form['sitestats_yahoo_backlink'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Yahoo Backlink?'),
        '#default_value' => variable_get('sitestats_yahoo_backlink', TRUE),
        );
      $form['sitestats_google_backlink'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Google Backlink?'),
        '#default_value' => variable_get('sitestats_google_backlink', TRUE),
        );
      $form['sitestats_technorati_rank'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Technorati Rank?'),
        '#default_value' => variable_get('sitestats_technorati_rank', TRUE),
        );
        $form['sitestats_google_indexed'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Google Indexed?'),
        '#default_value' => variable_get('sitestats_google_indexed', TRUE),
        );
      $form['sitestats_alltheweb'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Alltheweb Result?'),
        '#default_value' => variable_get('sitestats_alltheweb', TRUE),
        );
      $form['sitestats_yahoo_listed'] = array(
        '#type' => 'checkbox',
        '#title' => t('Listed In Yahoo Directory?'),
        '#default_value' => variable_get('sitestats_yahoo_listed', TRUE),
        );
      $form['sitestats_dmoz_listed'] = array(
        '#type' => 'checkbox',
        '#title' => t('Listed In DMOZ.org'),
        '#default_value' => variable_get('sitestats_dmoz_listed', TRUE),
        );
      $form['sitestats_domain_age'] = array(
        '#type' => 'checkbox',
        '#title' => t('Show Domain Age?'),
        '#default_value' => variable_get('sitestats_domain_age', TRUE),
        );
    

    It would be better to use the form field checkboxes.

17. //get alexa backlink
    function _sitestats_alexa_backlink($url) {
    	global $alexa_backlink;
    	if ($alexa_backlink!=0)
    	{
    		return $alexa_backlink;
    	} else {
    		$rank = _sitestats_get_alexa_popularity($url);
    		return $alexa_backlink;
    	}
    }
    
    //get alexa reach rank
    function _sitestats_alexa_reach_rank($url) {
    	global $alexa_reach;
    	if ($alexa_reach!=0)
    	{
    		return $alexa_reach;
    	} else {
    		$rank = _sitestats_get_alexa_popularity($url);
    		return $alexa_reach;
    	}
    }
    

    The code is not formatted as the coding standards suggest.

See the Drupal coding standards to understand how a module code should be written.

What is wrong with the following code?

  if($pos === false){} else{
    $pagerank = (int) substr($data, $pos + 9);
    return $pagerank;
  }

1.       $params['subject'] = t(variable_get('sitestats_mail_sub', SITESTATS_MAIL_SUB));
         $params['body'] = t(variable_get('sitestats_mail_body', SITESTATS_MAIL_BODY), $vars);
   

   The first argument of t() is a literal string, not a dynamic value; differently, the script to extract the strings to translate will not extract the string.

3.   if ( !db_result(db_query("SELECT * FROM {sitestats} WHERE hashelement='%s'",$r['hashelement']))) {
       $return = drupal_write_record('sitestats', $r);
     }
   

   db_result() is used when the query is selecting a single field of the table, not the full row; the code should use a different function.

4.     '#type' => 'textarea',
       '#title' => t("URL (%max max)", array('%max' => $max_url)),
   

   The title should say URLs (%max max); differently, users will probably understand that that is the length of the URL, not the maximum number of URLs they can insert.
   Are you sure that the placeholder %max is the one you want to use?

5.   $urls = array_map('trim', $urls);
     $urls = array_filter($urls, 'strlen');
   

   The coding standards also report to use Drupal Unicode functions; although, in this case it's enough to write array_filter($urls).

6.     $msg = $r['mail'].' is now registered to the pagerank update on '.$r['url'];
   

   Concatenating string to obtain a string that is then passed to t() is normally not a good idea (watchdog() passes the string to t()); $r['mail'], and $r['url'] needs to be sanitized, before to be output (t() does that, if it gets the right placeholders).

7. function sitestats_getstats($url) {
     $url = str_replace('www.','',$url);
     $url = str_replace('http://','',$url);
     $url = str_replace('https://','',$url);
   

   str_replace() accepts arrays as parameters; there isn't the need to call the function three times.

8.   $options = array(
         'PR' => "Show Google Pr?", 
         'AP' => "Show Alexa Popularity?", 
         'AB' => "Show Alexa Backlink?", 
         'GB' => 'Show Google Backlink', 
         'TR' => 'Show Technorati Rank?', 
         'GI' => 'Show Google Indexed?', 
         'AI' => 'Show Alltheweb Indexed?', 
         'DM' => 'Listed in DMOZ.org', 
         'DA' => 'Show Domain Age'
         );
   

   Every strings used in the user interface should be translated.
   In the options for the checkbox(ex) form field, the question mark is never used at the end of the string.
   Strings used in user interface should have the first word in capital case, and the other words in lower case (with the exception of proper nouns, adjectives derived from proper nouns, and acronyms).

9.       '#multicolumn' => array('width' => 3, 'row-major' => TRUE),
   

   That option is not used by Drupal core code; is it used by a third-party module?

10. // Run the javascript on page load.
    if (Drupal.jsEnabled) {
      $(document).ready(function () {
        $("input[@name=submit]").click(sitestats_handler);
        $('<div id="loading1"></div>').insertBefore('#target_sitestats').hide();
      }); 
    }
    

    Why isn't the code using Drupal behaviors?

The code is using some PHP functions when there are equivalent Drupal functions, which should be preferred.

urlencode() should be replaced by drupal_urlencode(), which fixes some Apache problems.