Kontroler, akcje ale trochę inaczej. Opcje

[Joachim Peters]

Witam,

Ostatnio trochę czytałem na temat MVC i tak naprawdę tylko niektóre rzeczy z tego mi się podobają, nie wiem może jeszcze nie dostrzegam za wielu plusów, ale chce zastosować u siebie coś podobnego. Napisałem sobie klasę router i request, pierwsza pobiera dane z adresu, druga z formularzy, cookies, sesji i obsługuje dane z routera, klasę FrontController, które uruchamia moduł i tworzy instancję i klasę Action, uruchamia akcję (wywołuje metodę klasy).
Kodowo wygląda to mniej więcej tak:

[PHP] pobierz, plaintext 
<?php
 
class FrontController {
	 public $module;
	 public $action;
	 public $objectAction;
	 
	public $request;
	 
	public function __construct(HttpRequest $request) {
		$this->request = $request;
 
		$this->module = strtolower($this->request->get('get', 0));
		$this->action = strtolower($this->request->get('get', 1));
 
		$this->runModule();
	}
 
	public function runModule() {
		if(file_exists('./modules/user/'.$this->module.'.php')) {
			include_once('./modules/user/'.basename($this->module.'.php'));
 
			$module = ucfirst($this->module);
 
			$this->objectAction = new $module();
		} else {
			throw new Exception('Nie można znaleźć plików modułu');
		}
	}	
}
 
class Action {
	 private $controller;
	 
	 public function __construct(FrontController $controller) {
		  $this->controller = $controller;
 
		$this->runAction();
	}
 
	public function runAction() {		
		if(!is_null($this->controller->objectAction)) {
			 $action = $this->controller->action;
			 
			$this->controller->objectAction->$action();
		} else {
			throw new Exception('Brak akcji');
		}
	}
}
 
$controller = new FrontController($request);
$action	= new Action($controller);
 
?>
[PHP] pobierz, plaintext 

Jak powiecie czy to rozwiązanie jest optymalne, warto to dalej kodować, czy lepiej zabrać się za kodowanie 'standardowe' - z wprowadzaniem argumentów do funkcji itd.?

[bim2]

Wedługmnie prawie OK. Chodzi o to, że w FrontController powinieneś dać new Action(); $oAction->$_GET['action']() cos w tym stylu, żeby to kontroller uruchamiał akcję. Aha i jeszcze opłacałoby się przenieść z akcji __construct do np SubController, dać extends SubController a w SubController dać te zmienne z Kontrollerem

[dr_bonzo]

Joachim: zakoduj to tak zeby przy tworzeniu serwisu na tym frameworku robilo sie to szybko (leniwie, niech robi jak najwiecej za ciebie, np. z ta akcja, po co masz to pisac za kazdym razem, niech to taki kontroller odpala) i przyjemnie i zebys za duzo kodu nie powtarzal

[Joachim Peters]

Mówisz, żeby metodę wywoływać kontroler tylko co wtedy z klasą Action, która będzie pusta, no chyba, że dam dziedziczenie klasy FrontController z Action i w kontrolerze wywołać metodę z $this?

[bim2]

Dziedziczenie. O tym pisalem. Ja zrobiłem sobie osobne klasy Application_Action/Model/View gdzie mam __construct() { $this->oController = FrontController::getInstance(); coś w tym stylu

[Joachim Peters]

Zamotałem się strasznie

[PHP] pobierz, plaintext 
<?php
 
class SubController {
	public $frontController;
 
	public function __construct(FrontController $controller) {
		$this->frontController = $controller;
	}
}
 
class Action extends SubController {
	public function runAction() {		
		if(!is_null($this->frontController->oAction)) {
			 $action = $this->frontController->action;
			 
			if(method_exists($this->frontController->oAction, $action)) {
				$this->frontController->oAction->$action();
			} else {
				throw new Exception('Moduł nie posiada takiej akcji');
			}
		} else {
			throw new Exception('Brak akcji');
		}
	}
}
 
$action = new Action;
 
class FrontController extends Action {
	 public $module;
	 public $action;
	 public $oAction;
	 
	public $request;
	 
	public function __construct(HttpRequest $request) {
		$this->request = $request;
 
		$this->module = strtolower($this->request->get('get', 0));
		$this->action = strtolower($this->request->get('get', 1));
 
		$this->runModule();
	}
 
	public function runModule() {
		if(file_exists('./modules/user/'.$this->module.'.php')) {
			include_once('./modules/user/'.basename($this->module.'.php'));
 
			$module = ucfirst($this->module);
 
			if(class_exists($module)) {
				$this->oAction = new $module();
 
				$this->runAction();
			} else {
				throw new Exception('Nie ma modułu o takiej nazwie');
			}
		} else {
			throw new Exception('Nie można znaleźć plików modułu');
		}
	}	
}
 
	$controller		= new FrontController($request);
	$subController			= new SubController($controller);
 
?>
[PHP] pobierz, plaintext 

Wszystko się teraz rozjechało, bo są złe powiązania. Jakieś rady ?

Ten post edytował Joachim Peters 31.08.2007, 09:43:06

[Ludvik]

Powiązania, jak sam zauważyłeś, są złe. Zacznijmy od tego, że Front Controller nie jest akcją - on wywołuje akcje, a to zupełnie inna relacja. Jeżeli chcesz podzielić działanie w stylu: moduł = klasa, akcja = metoda, to bym proponował stworzenie interfejsu modułu:

[PHP] pobierz, plaintext 
<?php
interface Module {
	function runAction($action);
}
?>
[PHP] pobierz, plaintext 

U Ciebie za moduł robił Sub Controller, ale nazwa Module wydaje się logiczniejsza moim zdaniem, szczególnie, że używasz jej w nazwie metody runModule(). Możesz pokusić się o stworzenie klasy abstrakcyjnej, albo zastąpić nią interfejs:

[PHP] pobierz, plaintext 
<?php
abstract class AbstractModule implements Module {
	public function runAction($action) {
		if(method_exists($this->frontController->oAction, $action)) {
			$this->frontController->oAction->$action();
		} else {
			throw new NoActionException();
		}
	}
}
?>
[PHP] pobierz, plaintext 

Front Controller jest ok, tylko usuń to dziedziczenie... Do tego bym odpalał obsługę żądania nie w konstruktorze, ale osobnej metodzie np. run().

[Joachim Peters]

Teraz mam coś takiego:

[PHP] pobierz, plaintext 
<?php
 
class FrontController {
	 public $module;
	 public $action;
	 public $oAction;
	 
	public $request;
	 
	public function __construct(HttpRequest $request) {
		$this->request = $request;
 
		$this->module = strtolower($this->request->get('get', 0));
		$this->action = strtolower($this->request->get('get', 1));
	}
 
	public function run() {
		$this->runModule();
	}
 
	private function runModule() {
		if(file_exists('./modules/user/'.$this->module.'.php')) {
			include_once('./modules/user/'.basename($this->module.'.php'));
 
			$module = ucfirst($this->module);
 
			if(class_exists($module)) {
				$this->oAction = new $module();
			} else {
				throw new Exception('Nie ma modułu o takiej nazwie');
			}
		} else {
			throw new Exception('Nie można znaleźć plików modułu');
		}
	}	
}
 
interface Module {
	public function runAction($action);
}
 
abstract class AbstractModule implements Module {
	  public $controller;
	 
	 public function __construct(FrontController $controller) {
		$this->controller = $controller;
	}
 
	public function runAction($action) {
		if(method_exists($this->controller->oAction, $action)) {
			$this->controller->oAction->$action();
		} else {
			throw new NoActionException();
		}
	}
}
 
$controller = new FrontController($request);
$module	 = new Module($controller);
 
$controller->run();
 
?>
[PHP] pobierz, plaintext 

i gdzie teraz wywołać tą akcję?

[Ludvik]

Wywołanie akcji powinno być w metodze runModule kontrolera:

[PHP] pobierz, plaintext 
<?php
private function runModule() {
	if(file_exists('./modules/user/'.$this->module.'.php')) {
		include_once('./modules/user/'.basename($this->module.'.php'));
 
		$module = ucfirst($this->module);
 
		if(class_exists($module)) {
			$oModule = new $module();
			  $oModule->runAction($this->action);
		} else {
			throw new Exception('Nie ma modułu o takiej nazwie');
		}
	} else {
		throw new Exception('Nie można znaleźć plików modułu');
	}
}
?>
[PHP] pobierz, plaintext 

Następnie musisz utworzyć same akcje, które wywiedziesz z klasy AbstractModule:

[PHP] pobierz, plaintext 
<?php
class HelloWorldModule extends AbstractModule {
	public function helloWorld() {
		 // Tutaj wykonuje sie akcja helloWorld() modułu HelloWorldModule
	}
}
?>
[PHP] pobierz, plaintext 

Żeby wszystko uruchomić, musisz wywołać metodę run() FrontControllera.

[Joachim Peters]

Mam teraz taki kod:

[PHP] pobierz, plaintext 
<?php
 
class FrontController {
	 public $moduleName;
	 public $actionName;
	 
	 public $oModule;
	 
	public $request;
	 
	public function __construct(HttpRequest $request) {
		$this->request = $request;
 
		$this->moduleName = strtolower($this->request->get('get', 0, 'htmlspecialchars'));
		$this->actionName = strtolower($this->request->get('get', 1, 'htmlspecialchars'));
	}
 
	public function run() {
		$this->runModule();
	}
 
	private function runModule() {
		if(file_exists('./modules/user/'.$this->moduleName.'.php')) {
			include_once('./modules/user/'.basename($this->moduleName.'.php'));
 
			$moduleName = ucfirst($this->moduleName);
 
			if(class_exists($moduleName)) {
				$this->oModule = new $moduleName();
				$this->oModule->runAction($this->actionName);
			} else {
				throw new Exception('Moduł <i>'.$this->moduleName.'</i> nie istnieje.');
			}
		} else {
			throw new Exception('Nie można znaleźć pliku modułu <i>'.$this->moduleName.'</i>.');
		}
	}	
}
 
interface Module {
	public function runAction($action);
}
 
class AbstractModule implements Module {
	  public $controller;
	 
	 public function __construct(FrontController $controller) {
		$this->controller = $controller;
	}
 
	public function runAction($action) {
		if(method_exists($this->controller->oModule, $action)) {
			$this->controller->oModule->$action();
		} else {
			throw new Exception('Akcja <i>'.$action.'</i> nie istnieje.');
		}
	}
}
 
	$controller		= new FrontController($request);
	$module			= new AbstractModule($controller);
// tutaj klasa modułu z 'extends AbstractModule
$controller->run();
?>
[PHP] pobierz, plaintext 

i pokazuje błąd Fatal error: Argument 1 passed to AbstractModule::__construct() must be an object of class FrontController...on line 47

[Ludvik]

Trochę nie tędy droga. Zacznijmy od front kontrolera, który ma właściwości publiczne - powinny być prywatne.

Usunąłeś słowo kluczowe abstract z definicji klasy AbstractModule, która miała być abstrakcyjna. Nie możesz utworzyć instancji tej klasy, bo ona nic nie robi... Posiada tylko metodę runAction, która na podstawie nazwy akcji wywołuje odpowiednią metodę. Te metody musisz już napisać sam w klasie dziedziczącej AbstractModule.

W przypadku tej konstrukcji, nie widzę sensu w podawaniu do konstruktora klasy modułu instancji front controllera. Na nic on nie jest potrzebny w tej klasie...

Metoda runAction() w AbstractModule jest źle napisana - po co wyciągać obiekt modułu z front controllera? Przecież front controllera wywołuje tę metodę dla odpowiedniego modułu, dlatego runAction() operuje na składowych instancji, dla której została wywołana. Dlatego powinno być:

[PHP] pobierz, plaintext 
<?php
public function runAction($action) {
	if(method_exists($this, $action)) {
		$this->$action();
	} else {
		throw new Exception('Akcja <i>'.$action.'</i> nie istnieje.');
	}
}
?>
[PHP] pobierz, plaintext 

Warto wyrzucić do tego jakiś lepszy wyjątek, typu NoActionException. Komunikat nic Ci tak na prawdę nie daje w kodzie. Po typie wyjątku możesz już manipulować przechwytywaniem...

Warto też do akcji jakoś przekazać obiekt żądania.

[Joachim Peters]

Mam teraz tak:

[PHP] pobierz, plaintext 
<?php
 
class FrontController {
	 private $moduleName;
	 private $actionName;
	 
	 private $oModule;
	 
	private $request;
	 
	public function __construct(HttpRequest $request) {
		$this->request = $request;
 
		$this->moduleName = strtolower($this->request->get('get', 0, 'htmlspecialchars'));
		$this->actionName = strtolower($this->request->get('get', 1, 'htmlspecialchars'));
	}
 
	public function run() {
		$this->runModule();
	}
 
	private function runModule() {
		if(file_exists('./modules/user/'.$this->moduleName.'.php')) {
			include_once('./modules/user/'.basename($this->moduleName.'.php'));
 
			$moduleName = ucfirst($this->moduleName);
 
			if(class_exists($moduleName)) {
				$this->oModule = new $moduleName();
				$this->oModule->runAction($this->actionName);
			} else {
				throw new Exception('Moduł <i>'.$this->moduleName.'</i> nie istnieje.');
			}
		} else {
			throw new Exception('Nie można znaleźć pliku modułu <i>'.$this->moduleName.'</i>.');
		}
	}	
}
 
interface Module {
	public function runAction($action);
}
 
abstract class AbstractModule implements Module {
	public function runAction($action) {
		if(method_exists($this, $action)) {
			$this->$action();
		} else {
			throw new Exception('Akcja <i>'.$action.'</i> nie istnieje.');
		}
	}
}
 
$controller = new FrontController($request);
$controller->run();
?>
[PHP] pobierz, plaintext 

Pytanie i problemy:
1. Chciałbym jeszcze zrobić, żeby w module było można używać funkcji i danych z klasy request bez potrzeby tworzenia instancji w każdej z klas gdzie ma być to użyte.
2. Nie bardzo rozumiem, jaki sens jest w tworzeniu typu wyjątku takiego jaki podałeś?
3. Co teraz z szablonami? Chce użyć zwykłej składni PHP w HTML-u w plikach .php i czy robić do wczytywania tego jakąś osobną klasę czy w metodach modułów dawać po prostu include?
4. Czy jestem taki toporny czy mi się tylko wydaję?

Ten post edytował Joachim Peters 31.08.2007, 15:55:27

[Ludvik]

1. Przekaż instancję klasy HttpRequest do metody runAction, a z tej do metody wykonującej akcję.
2. Popatrz na wywołanie metody run Front Controllera. Jedyne co będziesz mógł zrobić z tymi wyjątkami, to wyrzucić je na ekran. Wyjątki powinny mieć takie typy, aby można było wywnioskować z kontekstu, czego one dotyczą. Jak jedna z Twoich akcji wyrzuci wyjątek, to nie będziesz wiedział, czy nie ma modułu, czy akcji, czy też akcja nie zrobiła czegoś źle. Komunikat na poziomie kodu źródłowego nic nie daje...
3. Zdecydowanie trzeba wyodrębnić widok, jeżeli chcesz, żeby miało to coś wspólnego z MVC. Najprostsze podejście, to zwrócenie nazwy szablonu z akcji i przekazanie jej do obiektu widoku, który dołączy odpowiedni szablon.
4. Bez przesady Nikt nie rodzi się ze znajomością OOP, potrzeba trochę czasu, żeby poruszać się swobodnie w tym temacie. Trzeba też popełnić trochę błędów i wyciągnąć z nich wnioski

[Joachim Peters]

c.d 2. Nadal nie za bardzo rozumiem, załóżmy, że mają wyrzucić się dwa wyjątki - puste, skąd mam wiedzieć co wtedy jest nie tak? Czy chodzi Ci o to, żebym dawał tyle klas wyjątków ile jest ich wystąpień, czyli do każdego zdarzenia inny wyjątek (z inną nazwą)?
c.d 3. Ok, mniej więcej wiem jakby to miało wyglądać tylko co jeżeli dana akcja będzie posiadała wiele widoków? Czy będzie trzeba składować cały kod widoku tej akcji w jednym pliku i później będzie od wczytywany podczas uruchamiania akcji ($view->get()) ?

Ten post edytował Joachim Peters 31.08.2007, 18:29:55