Re: Normalized Numbers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Roman Neuhauser wrote:
> # jochem@xxxxxxxxxxxxx / 2007-01-12 01:57:27 +0100:
>> Brian P. Giroux wrote:
>>> If anyone can help me out with that or provide any other advice about
>>> the rest of it, I'd be grateful.
>>> The file can be found at http://www.senecal.ca/normnums.php.txt
> 
>> keep commenting all your code to that extent! you do us proud :-)
> 
> I find the *inline* comments superfluous and cluttering the code.

Because of my inexperience, I need the comments to remind me of what I
did a few days prior.

> They don't add any insight, they simply repeat what the code already says
> very succintly:
> 
>  1  function is_valid_isbn10_check_digit($cd) {
>  2    // check if th function was passed only a single character
>  3    if(1==strlen($cd)) {
>  4      // check if the digit is a valid ISBN-10 check digit
>  5      if(is_numeric($cd) || 'x'==$cd || 'X'==$cd) {
>  6        return true;
>  7      } else { // the digit is invalid
>  8        return false;
>  9      }
> 10    } else { // the check digit isn't 1 character
> 11      return false;
> 12    }
> 13  }
> 
> Comments on lines #2, #4, #7, #10 only restate painfully obvious things.
> Who needs to explain that "13 == strlen($ean)" checks that the length of
> $ean is 13?
> 
> This shorter version is more readable for me:
> 
>  1  function is_valid_isbn10_check_digit($cd)
>  2  {
>  3      return (1 == strlen($cd)
>  4          && (is_numeric($cd) || 'x'==$cd || 'X'==$cd)
>  5      );
>  6  }

WOW! I knew the functions could be whittled down (although I couldn't
see how) but I never thought it could be just be a single statement.

> The code is quite complicated for no good reason I could see:
> 
>  1  function is_valid_ean($ean) {
>  2    //check that the string is 13 characters long
>  3    if(13==strlen($ean)) {
>  4      // make sure all digits are numeric
>  5      if(is_numeric($ean)) {
>  6        if(0==digit_sum($ean,1,1,3)%10) {
>  7          return true;
>  8        } else { return false; }
>  9      } else { return false; }
> 10    } else { return false; }
> 11  }
> 
> First step:
> 
>  1  function is_valid_ean($ean) {
>  2    if(13==strlen($ean)) 
>  3      if(is_numeric($ean))
>  4        if(0==digit_sum($ean,1,1,3)%10)
>  5          return true;
>  6    return false;
>  7  }

This makes sense to me, I had forgotten that "return" exits the function
immediately, making the "else" statements unnecessary.

> Second step:
> 
>  1  function is_valid_ean($ean) {
>  2    if(13==strlen($ean)
>  3      && is_numeric($ean)
>  4      && (0==digit_sum($ean,1,1,3)%10))
>  5          return true;
>  6    return false;
>  7  }

This also makes sense now that the "else" statements have been removed.

> Third step:
> 
>  1  function is_valid_ean($ean) {
>  2    return (13 == strlen($ean)
>  3        && is_numeric($ean)
>  4        && (0 == (digit_sum($ean,1,1,3) % 10))
>  5    );
>  6  }

Again, WOW! This is certainly the version I will use (if you don't mind).

> The last version tells me what I need to know, and tells it only once!
> The three lines are so little of so "uninteresting" code, (there's
> obviously nothing overly complicated going on) that they don't need more
> explanation than a good function name provides.
> 

-- 
Brian P. Giroux

-- 
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php


[Index of Archives]     [PHP Home]     [Apache Users]     [PHP on Windows]     [Kernel Newbies]     [PHP Install]     [PHP Classes]     [Pear]     [Postgresql]     [Postgresql PHP]     [PHP on Windows]     [PHP Database Programming]     [PHP SOAP]

  Powered by Linux