[dev] [commit] r990 - wwwbase

Cătălin Frâncu cata at francu.com
Fri Sep 20 03:00:14 EEST 2013


Arată bine în general, dar mi-e greu să-l rulez momentan. Nu am tabelele, fiindcă nu le-ai adăugat la patches/, și pare că lipsește un fișier .ihtml (cred).

Cum am mai zis, nu uita să adaugi mesaje de commit. Reflexele prost formate nu se mai dezvață. :-)

> +require_once '../phplib/util.php';
> +require_once '../phplib/serverPreferences.php';
> +require_once '../phplib/db.php';
> +require_once '../phplib/idiorm/idiorm.php';
> +require_once '../phplib/idiorm/paris.php';

Numai prima linie este necesară aici, căci util.php include tot ce este nevoie.

> +db_init();

La fel, asta se întâmplă din util.php.

> +
> +	function getNextOffset() {
> +		crawlerLog("INSIDE " . __FILE__ . ' - ' . __CLASS__ . '::' . __FUNCTION__ . '() - ' . 'line '.__LINE__ );
> +		while($this->currOffset <= $this->textEndOffset) {
> +			//daca urmatorul offset e a,i,s,t sau ă,â,î,ș,ț
> +			if (self::isPossibleDiacritic(StringUtil::getCharAt($this->text, $this->currOffset))) {
> +				return $this->currOffset ++;
> +			}
> +			$this->currOffset ++;
> +		}
> +		return null;
> +	}

Aș vrea să evităm programarea nestructurată aici. Un return în mijlocul funcției face codul mai greu de citit. Cred că se poate rescrie așa:

while($this->currOffset <= $this->textEndOffset && !self::isPossibleDiacritic(StringUtil::getCharAt($this->text, $this->currOffset))) {
   $this->currOffset ++;
}

return ($this->currOffset < $this->textEndOffset) ? $this->currOffset : null;

Este un algoritm clasic de căutare într-un șir de date și ochiul îl recunoaște ca atare:

cât timp (mai am elemente && elementul curent nu este cel căutat) { treci la elementul următor }.

> +	function toLower($content) {
> +		crawlerLog("INSIDE " . __FILE__ . ' - ' . __CLASS__ . '::' . __FUNCTION__ . '() - ' . 'line '.__LINE__ );
> +		return mb_strtolower($content);
> +	}

Mai este nevoie de funcția asta? Dacă nu plănuiești să o extinzi într-un viitor, eu aș zice să o ștergi și să folosești direct mb_strtolower().

> +	static function isPossibleDiacritic($ch) {
> +		crawlerLog("INSIDE " . __FILE__ . ' - ' . __CLASS__ . '::' . __FUNCTION__ . '() - ' . 'line '.__LINE__ );
> +		return strstr(self::$nonDiacritics, $ch);
> +	}

Aici poate sunt eu amețit, dar dacă $ch este diacritic, atunci ar trebui să NU existe în $nonDiacritics, nu?

De fapt, văd că în crawler_dex.conf nici nu este definită valoarea non_diacritics, este ok așa?

> +		crawlerLog("WTF " . $key);

Tot ce ține de nedumerirea programatorului ar trebui să nu ajungă în repository. :-)

> +
> +if (strstr( $_SERVER['SCRIPT_NAME'], 'diacritice.php')) {

Nu prea înțeleg ce face linia asta (dar nici nu-mi amintesc detaliile despre variabila $_SERVER). Nu e garantat că SCRIPT_NAME va fi diacritice.php?

> +	if (isset($_POST['text']) && $_POST['text'] != '') {
> +
> +		$obj = new DiacriticsFixer();
> +		SmartyWrap::assign('result', $obj->fix($_POST['text']));

Folosește și aici $text = util_getRequestParameter('text'). Ca principiu, e bine să ai un sistem și să te ții de el, pentru că, dacă vrei să schimbi ceva, trebuie să schimbi într-un singur loc (în funcția util_getRequestParameter() în acest caz).

> +	SmartyWrap::displayPageWithSkin('../diacritics_fix/diacritics_fix.ihtml');

Pare că nu ai făcut commit și la diacritics_fix.ihtml.


More information about the Dev mailing list