[php]Początki OOP Opcje

[Mefiuu]

Witam. Rozpocząłem żmudne zagłębianie się w tajniki programowania zorientowanego obiektowo, przeczytałem kilka artykułów i pomyślałem że najlepiej jest sprawdzić swoją wiedzę w praktyce. Tak oto powstała klasa do uploadu plików. Przyznam się, że pobrałem sobie podobną klasę z phpclasses.org, aby zobaczyć mniej więcej jak to wygląda. Chciałbym Was zapytać czy ma to coś w ogóle wspólnego ze stylem OOP ?

class.upload.php

[PHP] pobierz, plaintext 
<?php
class Upload {
	var $folder;
	var $types = array('application/msword', 'application/octet-stream', 'image/jpeg'); 
 
	function __construct($folder) {
		$this->folder = $folder;
		$this->upload();
	}
 
	public function check() {
            if (isset($_POST['wyslij'])) {
		if($_FILES['plik']['error']>0) {
			echo "Wystąpił problem: ";
			switch($_FILES['plik']['error']) {
				case 1: echo "rozmiar pliku przekroczył wartość xMB.<br /><a href='java script:history.back(1)'>Wróć</a>"; break;
				case 2: echo "rozmiar pliku przekroczył wartość xMB.<br /><a href='java script:history.back(1)'>Wróć</a>"; break;
				case 3: echo "plik wysłany częściowo.<br /><a href='java script:history.back(1)'>Wróć</a>"; break;
				case 4: echo "nie wysłano żadnego pliku.<br /><a href='java script:history.back(1)'>Wróć</a>"; break;
				case 6: echo "nie można wysłać pliku: nie wskazano katalogu tymczasowego.<br /><a href='java script:history.back(1)'>Wróć</a>"; break;
				case 7: echo "nie zapisano pliku na dysku.<br /><a href='java script:history.back(1)'>Wróć</a>"; break;
				}
			}
		else {
			if (in_array($_FILES['plik']['type'], $this->types)) {
				return true;
			}
		}
	}
}
 
public function upload() {
		if($this->check()) {
			if (is_uploaded_file($_FILES['plik']['tmp_name'])) {
					if(!file_exists($this->folder.'/'.$_FILES['plik']['name'])) {
						if(move_uploaded_file($_FILES['plik']['tmp_name'], $this->folder.'/'.$_FILES['plik']['name'])) {
								throw new Exception('Dodano plik pomyślnie');
					}
					else {
						throw new Exception('Nie dodano pliku!');
					}
				}
				else {
					throw new Exception('Taki plik już istnieje!');
				}
			}
		}
		else {
			throw new Exception('Niepoprawny typ!');
		}
	}
}
?>
[PHP] pobierz, plaintext 

index.php

[PHP] pobierz, plaintext 
<?php
 
include('class.upload.php');
 
try {
	$upload = new Upload('files');
}
catch (Exception $e) {
		echo $e->getMessage();
}
 
?>
[PHP] pobierz, plaintext 

Mam co do tego pytania :

1) Wiem że w stylu OOP są wyjątki. Czy to, jak ja je zastosowałem to odpowiednia metoda? Czy jest ich za dużo ? Bo mi nie pasują za bardzo ...
2) Czy wyjątki powinno się dawać do udanych operacji ?
3) Czy w konstruktorze wykonywać metodę 'upload' czy odwołać się do niej poza konstruktorem, już w pliku index?
4) Jeśli mam operacje na bazie danych bądź plikach tekstowych to czy tworzyć destruktor do ich zamykania ?

To takie ogólne pytania i byłbym bardzo wdzięczny gdyby ktoś mi to 'sprawdził', wytknął błędy i polecił dobrą literaturę (IMG:style_emoticons/default/smile.gif)

Dziękuję i pozdrawiam.

Ten post edytował Mefiuu 3.08.2011, 20:46:10

[Crozin]

Na początek dwie ogólne uwagi:
1. Nie mieszaj języków (patrz: wywal polskie indeksy tablic). Angielski jest jedynym słusznym w takim miejscu. (IMG:style_emoticons/default/wink.gif)
2. Nigdy nie doprowadzaj do czegoś takiego:

[PHP] pobierz, plaintext 
if () {
	if () {
		if () {
			if () {
 
			} else {
 
			}
		} else {
 
		}
	} else {
 
	}
} else {
 
}
[PHP] pobierz, plaintext 

Tyle poziomów zagłębień to koszmar przy późniejszej pracy. Niemal zawsze da się to zamienić na coś w stylu:

[PHP] pobierz, plaintext 
// Obsługa czterech "błędów"
if (!...) {
 
}
 
if (!...) {
 
}
 
if (!...) {
 
}
 
if (!...) {
 
}
 
// Obsługa po pomyślnym przejściu przez powyższe
...
[PHP] pobierz, plaintext 

Pamiętaj, że wyrzucenie wyjątku przerywa wykonywanie bloku TRY.

Co do Twoich pytań:
1. Nie jest ich za dużo. W każdym miejscu gdzie występuje błąd, który wystąpić nie powinien masz prawo, a wręcz obowiązek skorzystać z wyjątku.
2. Nie.
3. Nie powinieneś. Konstruktor służy przygotowaniu obiektu do pracy, nie wykonywaniu samej pracy.
4. To zależy. Jeżeli plik jest otwarty na przestrzeni "działania obiektu" to w destruktorze dobrze byłoby upewnić się, że plik zostanie zamknięty. Przy czym raczej powinna być osobna metoda do zamknięcia, użytkownik powinien zawsze ręcznie ją wywoływać, a destrukor mógłby co najwyżej w ostateczności zamykać. Coś w ten deseń:

[PHP] pobierz, plaintext 
class File {
  private $opened = false;
 
  public function open(...) {
     ...
 
     $this->opened = true;
  }
 
  public function write(...) {
     ...
  }
 
  public function close() {
     ...
 
     $this->opened = false;
  }
 
  public function __descruct() {
    if ($this->opened) {
      $this->close();
    }
  }
}
[PHP] pobierz, plaintext 

I jeszcze kilka uwag:
1. Nazwa klasy powinna być rzeczownikiem, np. Uploader. Tworząc obiekt (new) tworzysz jakiś byt, a następnie każesz owemu bytowi wykonywać jakieś akcje, np. wgrać coś ($uploader->uploadSomething()) dlatego też nazwy metod powinny być czasownikami. Kod jest wtedy bardziej logiczny.
2. W PHP5 nie ma czegoś takiego jak "var". Masz trzy modyfikatory zasięgu: private, protected, public i to z nich powinieneś korzystać.
3. Nigdy nie powinieneś korzystać ze stanu globalnego (patrz $_POST, $_FILES). Te dane powinny zostać przekazane do obiektu z zewnątrz.
4. Akurat na kod błędu (1..7) masz ładne stałe, które są bardziej wymowne niż jakiś numerek.
5. Nigdy nie powinieneś mieć w kodzie takiej klasy żadnego ECHO. Jest błąd? To wywalasz wyjątek. Jakaś inna część aplikacji może go przechwycić i zrobić coś pożytecznego z nim (jak chcociażby wyświetlenie informacji użytkownikowi).
6. Nigdy nie stosuj bezpośrednio klasy Exception. Jest zbyt ogólna. Użyj bardziej wyspecjalizowanego typu. A jeżeli w SPL-u nie ma odpowiedniego, stwórz własny.
7. Jak już przechwytujesz wyjątek (blok CATCH) zrób z nim coś pożytecznego. Wyświetlenie treści wyjątku nie jest czymś takim. Co najwyżej całą aplikację możesz objąć blokiem TRY .. CATCH i tutaj wyświetlenie treści błędu ma sens - bo oznacza to, że aplikacja się wysypała i nic już nie da się zrobić.