Witam

Napisalem klase do obslugi wyjątków, która przechwycone komunikaty, kod błędu i ślad wywołań zapisuje do pliku.

Prosiłbym o komentarze, co możnaby zrobić lepiej, co zmienić, itp. jakiejś dodatkowej funkcjonalności raczej nie przewiduje, chyba, że bedzie to naprawdę ciekawy pomysł.

Porad chciałbym zasięgnąć od doświadczonych programistów

A o to kod klasy:

[PHP] pobierz, plaintext 
<?php
 
define('EXCEPTION_CRITICAL', 1);
define('EXCEPTION_ERROR', 2);
define('EXCEPTION_WARNING', 4);
define('EXCEPTION_NOTICE', 8);
define('EXCEPTION_INFO', 16);
define('EXCEPTION_DEBUG', 32);
 
define("EXCEPTION_TARGET_DIR", "./logs");
define("EXCEPTION_LOGGING_LEVEL", EXCEPTION_DEBUG);
define("EXCEPTION_LENGTH_TYPE_STRING", 15);
 
class LoggedException extends Exception {
	protected $message="Unknown exception";
	protected $target_file="exc_main.txt";
	protected $target_path;
	protected $handled_file;
	protected $min_level;
	protected $level;
 
	public function __construct($message=null, $level=EXCEPTION_ERROR, $code=null) {
		parent::__construct($message, $code);
		$this->target_path=(defined("EXCEPTION_TARGET_DIR")?EXCEPTION_TARGET_DIR:'.')."/";
		$this->min_level=defined("EXCEPTION_LOGGING_LEVEL")?EXCEPTION_LOGGING_LEVEL:EXCEPTION_ERROR;
		$this->level=$level;
		if(($file_handle=$this->file_handlers($this->target_file)) === false) {
			$this->handled_file=fopen($this->target_path.$this->target_file, 'a');
			$this->file_handlers($this->target_file, $this->handled_file);
		} else {
			$this->handled_file=$file_handle;
		}
		if(is_writeable($this->target_path.$this->target_file) && is_resource($this->handled_file)) {
			$this->WriteMsg("Exception '".get_class($this)."' with message '$this->message (code: $this->code)' in $this->file($this->line)", $this->level);
			if(EXCEPTION_DEBUG <= $this->min_level) {
				$backtrace=$this->getTrace();
				$this->WriteMsg("Stack trace (".(sizeof($backtrace)+1)."):", EXCEPTION_DEBUG);
				for($i=0; $i < sizeof($backtrace); $i++) {
					$args_str="";
					foreach($backtrace[$i]['args'] AS $var) {
						$args_str.=$this->getTypeAsString($var).", ";
					}
					$args_str=substr($args_str, 0, -2);
					$this->WriteMsg("#$i ".$backtrace[$i]['file']."(".$backtrace[$i]['line']."): ".$backtrace[$i]['class'].$backtrace[$i]['type'].$backtrace[$i]['function']."($args_str)", EXCEPTION_DEBUG);
				}
				$this->WriteMsg("#$i {main}", EXCEPTION_DEBUG);
			}
		}
 
	}
	final protected function WriteMsg($msg, $level=EXCEPTION_INFO) {
		if(is_resource($this->handled_file)) {
			if($level <= $this->min_level) {
				$time=strftime('%d.%m.%Y %H:%M:%S', time());
				$str_level=$this->LevelToString($level);
				$msg=str_replace("t", "	", str_replace(array("rn", "n", "r"), " ", $msg));
				fwrite($this->handled_file, "$timet$str_levelt$msgn");
			}
		}
	}
	final protected function file_handlers($file, $handle=null) {
		static $handlers;
		if(!isset($handlers)) $handlers=array();
		if(is_null($handle)) {
			if(isset($handlers[$file])) {
				return $handlers[$file];
			} else {
				return false;
			}
		} else {
			if(isset($handlers[$file])) {
				return false;
			} else {
				$handlers[$file]=$handle;
				return true;
			}
		}
	}
	final public function getLevel() {
		return $this->level;
	}
	final public function getLevelAsString() {
		return $this->LevelToString($this->level);
	}
	final protected function LevelToString($level) {
		$list=array(
			EXCEPTION_CRITICAL => "EXCEPTION_CRITICAL",
			EXCEPTION_ERROR => "EXCEPTION_ERROR",
			EXCEPTION_WARNING => "EXCEPTION_WARNING",
			EXCEPTION_NOTICE => "EXCEPTION_NOTICE",
			EXCEPTION_INFO => "EXCEPTION_INFO",
			EXCEPTION_DEBUG => "EXCEPTION_DEBUG"
		);
		return isset($list[$level])?$list[$level]:"[unknown]";
	}
	final protected function getTypeAsString($var) {
		if(is_bool($var)) {
			return "bool(".($var===true?"true":"false").")";
		} else if(is_integer($var)) {
			return "int($var)";
		} else if(is_float($var)) {
			return "float($var)";
		} else if(is_double($var)) {
			return "double($var)";
		} else if(is_real($var)) {
			return "real($var)";
		} else if(is_null($var)) {
			return "NULL";
		} else if(is_string($var)) {
			$val=(defined("EXCEPTION_LENGTH_TYPE_STRING") && strlen($var) > EXCEPTION_LENGTH_TYPE_STRING)?substr($var, 0, EXCEPTION_LENGTH_TYPE_STRING)."[...]":$var;
			return "string(".strlen($var).") '".str_replace("t", "t", str_replace(array("rn", "n", "r"), "n", $val))."'";
		} else if(is_resource($var)) {
			return "resource '".substr($val=strval($var), strrpos($val, "#")+1)."' of type '".get_resource_type($var)."'";
		} else if(is_array($var)) {
			$array_str="{ ";
			foreach($var AS $key=>$val) {
				$array_str.="[$key] => ".$this->getTypeAsString($val).", ";
			}
			$array_str=substr($array_str, 0, -2)." }";
			return "array(".sizeof($var).") $array_str";
		} else if(is_object($var)) {
			return "object of class '".get_class($var)."'";
		} else {
			return "unknown type '$var'";
		}
	}
	final public function __destruct() {
		if(is_resource($this->handled_file)) {
			fclose($this->handled_file);
		}
	}
}
 
?>
[PHP] pobierz, plaintext 

Pozdrawiam

do oceniania jest osobne forum...
a co do klasy wygląda ok tylko metoda getTypeAsString z tymi wszystkimi if'ami wygląda dziwnie, popatrz na funkcję http://pl.php.net/manual/en/function.gettype.php

Witam

Znam wlasnie ta funkcje, tylko no niestety, nie zaspokoila moich potrzeb.
Jak widac, wraz z typem danych zwracam rowniez jakies informacje, np rozmiar tekstu, zawartosc tablicy. Chcialbym zachowac funkcjonalnosc tej metody taka jaka jest, a nie wiem, jak da sie to zrobic w inny sposob.

Pozdrawiam, no i komentarze nadal mile widziane

A zastosowanie gettype w czym ci przeszkadza ?
U mnie to wygląda tak.

[PHP] pobierz, plaintext 
<?php
public function parseArgs($args)
{
	$arg = '';
	foreach($args as $val) 
	{
		$type = gettype($val);
		if($type == 'boolean') {
			if($val) {
				$arg .= 'true, ';
			} else {
				$arg .= 'false, ';
			}
		} else if($type == 'string') {
			$arg .= "'$val', ";
		} else if($type == 'array') {
			$arg .= 'array('.count($val).'), ';
		} else if($type == 'object') {
			$arg .= 'object('.get_class($val).'), ';
		} else if($type == 'resource') {
			$arg .= 'resource('.get_resource_type($val).'), ';
		} else {
			$arg .= $val.', ';
		}
	}
	return substr($arg,0,strlen($arg)-2);
}
?>
[PHP] pobierz, plaintext 

metoda getBacktrace() korzysta z tej metody sprawdzając typ zmiennej.
Poza tym twój kod jest mało czytelny zobacz czy nie da się tego rozbić na kilka innych metod.
I jeszcze jedna uwaga może to jest lekka paranoja,
ale konstrukcja alternatywna if ( warunek ? spełniony : niespełniony ) oprócz tego że gorzej się ją czyta wykonuje się dłużej od zwykłej.
( czy ktoś może dodać stripslashes to dej cholernej funkcji wyświetlającej kod PHP na forum bo zaczyna być to irytujące ? )

Po pierwsze - po co tworzysz stałe globalne?? Nie prościej tworzyć stałe w klasie?
Po drugie zamieniłbym stałą Exception_Logging_Level na właściwość statyczną klasy.
Po trzecie - na mój gust metoda __construct jest przeładowana - dokonaj refaktoryzacji
Po czwarte - czemu wszystkie metody są final
Po piąte - jakoś nie odpowiada mi trzymanie logowania wyjątków w klasie wyjątków - jest to bliskie tworzenia boskich klas
Po szóste - zamiast tego dużego if'a użyłbyś var_export" title="Zobacz w manualu PHP" target="_manual

Dziwię się, że tak późno to padło. Obiekt wyjątku powinien pełnić swoją rolę, czyli służyć do zbierania informacji o sytuacji, w której został wyrzucony. Logowanie to zadanie dla zupełnie innego obiektu. Lepiej nic nie mieszaj z wyjątkami, klasa Exception jest na tyle dobra jako baza, że tworzenie nowych typów wyjątków ma za zadanie nadać im odpowiednią nazwę. Nic więcej...

Pomijając fakt, że w ten sposób jesteś na sztywno związany z jednym sposobem logowania wyjątków. Nie zmienisz implementacji, bo:

• musisz zmienić kod w klasie wyjątku
• albo musisz zmienić nazwę wyjątku we wszystkich miejscach, w których wystąpiła

Utworzenie obiektu, który będzie logował wyjątki, jest dużo lepszym rozwiązaniem.

Witam

Dzieki chlopaki, za ostra jadke

@UDAT

Co do definiowania zmiennych globalnych, od linii 3-8 niezmienie tego raczej, poniewaz, jesli tworze nowy wyjatek, to zdecydowanie bardziej przejrzyste jest uzycie takiego kodu:



niz takiego



jesli chodzi o nastepne 3 linijki definicji, to mam taki zcentralizowany plik konfiguracyjny (parsowanie pliku .ini) i objekt ktory to zczytuje, nastepnie definiuje wlasnie takie zmienne

Co do rozdzielenia klasy na klase do wyjatkow i do logowania myslalem nad tym, mam nawet gotowy logger, nie wiem, jakos mi to tak przyszlo, zeby zebrac to do jednej kupy, gdyz przychodzily mi do glowy rzeczy typu, a co jesli logger sie nie zaladuje, a to to, a to tamto, ale jednak po cos jest to OOP.

No nic, posiedze jeszcze nad tym i wielkie dzieki za porady

Jeszcze parę słów na koniec:
Ostatnio z coraz większym zainteresowaniem poznaję możliwości użycia AOP do poprawy jakości, które także w tym wypadku byłoby najlepszym rozwiązaniem. Oto szybko naskrobany aspekt:

[PHP] pobierz, plaintext 
<?php
class MyLogger  {
	public static function Log ( $message, Exception $e );
		echo $message, PHP_EOL;
		var_dump ( $e );
		echo  PHP_EOL;
	}
}
aspect LoggedException{
 
   public $instances = array(); //Trzyma unikalne instancje
 
   pointcut exc:new(Exception+(*));
 
 
   around(): exc{
	$exceptionObject = proceed();
	MyLogger::Log ( "Catched exception ", $exceptionObject );
	return $exceptionObject;
   }
}
?>
[PHP] pobierz, plaintext 

Powyższy przykład zawiera wyekstrahowany z klasy wyjątku - kod odpowiedzialny za logowanie. Oczywiście klasę MyLogger musisz utworzyć odpowiednią do zastosowania.

Natomiast zamiast odpowiednich poziomów(podawanych w konstruktorze) wyjątków, które są niezbyt praktyczne, używać dziedziczenia, np.:

[PHP] pobierz, plaintext 
<?php
class InfoException extends Exception{}
  class ErrorException extends Exception{}
  class FatalException extends Exception{}
?>
[PHP] pobierz, plaintext 

Przykład napisany do użycia z phpAspect - opis instalacji jest na stronie projektu i na moim blogu ( patrz Automatyczny Singleton ). Zapraszam do poznawania tego podejścia do programowania.