Re: cvs: pear /Validate/Validate IS.php

Bonjour, Hannes Magnusson wrote:

bjori Thu Nov 17 10:55:55 2005 EDT

  Modified files:                  /pear/Validate/Validate	IS.php   Log:

Fixed typo, thanks to Mark Wiesemann http://cvs.php.net/diff.php/pear/Validate/Validate/IS.php?r1=1.13&r2=1.14&ty=u Index: pear/Validate/Validate/IS.php diff -u pear/Validate/Validate/IS.php:1.13 pear/Validate/Validate/IS.php:1.14 --- pear/Validate/Validate/IS.php:1.13 Thu Nov 17 07:04:34 2005 +++ pear/Validate/Validate/IS.php Thu Nov 17 10:55:53 2005 @@ -23,7 +23,7 @@

  * @author     Hannes Magnússon <bjori@php.net>

* @copyright 1997-2005 Hannes Magnússon

  * @license    http://www.opensource.org/licenses/bsd-license.php  new BSD
- * @version    CVS: $Id: IS.php,v 1.13 2005/11/17 12:04:34 bjori Exp $
+ * @version    CVS: $Id: IS.php,v 1.14 2005/11/17 15:55:53 bjori Exp $

*/ /** @@ -153,7 +153,7 @@

                  static $postCodes = array();
         static $lastUrl   = '';
-        if ($strongi && $lastUrl != $url) {
+        if ($strong && $lastUrl != $url) {
             $lastUrl = $url;
             if (!$url) {
                 $url = "http://www.postur.is/gogn/Gotuskra/postnumer.txt";

Congrats, I think it's quite ok for the static "cache" However: 1) Your comment in header does not reflect the real mechanism, and at least should explain that if the given file is writable, and strong is asked, then the file will save what is obtained from the url. 2) Default values for parameters, please, use the PHP mechanism (which is the same as C):

    function postalCode($postcode, $strong = false, $dataDir = '@DATADIR@/Validate_IS', $url = 'http://www.postur.is/gogn/Gotuskra/postnumer.txt')
    {

3) For the file, I would not only let the path to the user, but the whole location as a file parameter instead:

    function postalCode($postcode, $strong = false, $file = '@DATADIR@/Validate_IS/IS_postcodes.txt', $url ...
For example, I could have a special file to validate it's in Reykjavik's area.

4) The access to the file (is_readable) should be donne only if necessary, not to do if same file, same url and $postCodes is not empty It should be done instead of the last file_exists(). 5) Moreover, as you have a complicated mechanism, depending on the file and optionnaly on the url, you need also to check the file name wasn't changed (using a static $lastFile='') the same as for url. Also, I usually park all the static variables used in top of the method so the mechanisms are more clear to understand. (they are perverse) *** function address() *** Avoid totally $this, Validate's class are normally purely static so self:: ? or Validate_IS:: are the onliest way, works in anycase. Hahaha, don't tell I'm faddy :) sorry to be so boring. a+ -- toggg