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

[snip] > > 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. Fixed. 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') > { I *was* going to impliment fallback mechanism wich is why I didn't assign any default values since it didnt make any sens to duplicate the default value. However, I drop that idea, if the user provide fucked datafile/url then its his fault and we now return false. 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. My fault. fixed :) 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(). fixed 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. fixed 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) Yeah, I usually do that to. Don't know why I didnt. fixed :) *** function address() *** > Avoid totally $this, Validate's class are normally purely static so > self:: ? or Validate_IS:: are the onliest way, works in anycase. No way in hell Im going to change this. I'd rather drop PHP4 support then changing this. Those who like to run E_STRICT at least have a change now by creating instance of the class now. Hahaha, don't tell I'm faddy :) sorry to be so boring. > a+ > -- > toggg > ;) - Hannes