Prosím o kritiku skriptů

Dobrý den
nedávno jsem dospěl k bodu, kdy jsem si řekl, že bych se mohl naučit psát MVC, nebo spíše Model2 (jak je ctěná libost) aplikace. Však jsem tu i žádal o nějaké prameny.
Nuže napsal jsem si velmi komplexní aplikaci - kalkulačku, která umí sčítat, odčítat a násobit. Zde předkládám kód a prosím o kritiku, ale nerad bych skončil s brekem pod postelí :-). A ano, vím že je poněkud úsměvné psát to v mvc když by jinak stačily tak 3 řádky. Ale nějak jsem začít musel. Popravdě samotný princip MVC už se mi zdá naprosto zřejmý, jen ho ještě správně implementovat.

index.php:

PHP kód:

<meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> 

<?php
 require('model.php');
require('controller.php');
require('view.php');



?>

VIEW

PHP kód:

<p>mvc aplikace - kalkulačka</p>


<a href="index.php?operation=plus">Sčítačka</a>
<a href="index.php?operation=minus">Odčítačka</a>
<a href="index.php?operation=multiplication">Násobička</a>

<?php
$form = <<<FORM
<form action="index.php" method="get">
    <input type="hidden" name="operation" value="$op">
    <input type="text" name="number1">
    <input type="text" name="number2">
    <input type="submit" value="Provést výpočet">
</form>
FORM;
if(isset($_GET['operation'])){
    echo $form;
    }
?>
<p>
<?php echo $choose; ?>
</p>

<p> <?php echo $print_res; ?> </p>

CONTROLLER

PHP kód:

<?php
$print_res = NULL;
if(isset($_GET['operation'])){
    $op = $_GET['operation'];
        
    
    switch($_GET['operation']){
        case "plus":
            $choose = "Vybrali jste sčítání";
        break;

        case "minus":
            $choose = "Vybrali jste odečítání";
        break;
    
        case "multiplication":
             $choose = "Vybrali jste násobení";
        break;

    }
    
    }else{
            $op = NULL;
            $choose = "Nebyla vybrána operace";
            
            }





if(isset($_GET['number1']) && isset($_GET['number2'])){
    $n1 = $_GET['number1'];
    $n2 = $_GET['number2'];
        $answ = new HardCoreMath($n1, $n2);

    switch($_GET['operation']){
        case "plus":
            $answ->plus();
            $print_res = "Výsledek: ".number_format($answ->plus_result, 3 , ',' , ' ');
        break;
    
        case "minus":
            $answ->minus();
            $print_res =  "Výsledek: ".$answ->minus_result;
        break;

        case "multiplication":
            $answ->multiplication();
            $print_res =  "Výsledek: ".$answ->multiplication_result;
        break;
            

    
    }//switch

}
?>

MODEL

PHP kód:

<?php
class HardCoreMath{
    public $number1;
    public $number2;

    function __construct($number1, $number2){
        $this->number1 = $number1;
        $this->number2 = $number2;

    }//__construct
    
    function plus(){
        $this->plus_result = $this->number1 + $this->number2;
         
        
    }//plus

    function minus(){
        $this->minus_result = $this->number1 - $this->number2;

    }//minus

    function multiplication(){
        $this->multiplication_result = $this->number1 * $this->number2;
    
    }



}//Hard_Core_Math class

?>

V duchu mvc model2 by ale model mel obsahovat "vysledek" a nemel by ho vracet, controller by nemel vybirat "textovou hlasku", muze ten text maximalne prelozit - ikdyz i to je nekde mezi view a controllerem, ale opet by mel byt na modelu.

Jinak to vypada celkem spravne - controller rozhoduje, co se zavola, model to provede a view to zobrazi.

A ano, je to naprosto zbytecne, protoze hlavni smysl rozdelovani na 3 vrstvy je v tom, ze je lze lehce vymenit - jine zobrazeni, jiny model, jine "osetrovani" requestu, coz se vetsinou nedeje a mame jenom jedno "view" pro kazdy model, jenom jeden model a jenom jeden zpusob osetrovani requestu.

Takže jestli správně rozumím, tak řádek:
$print_res = "Výsledek: ".$answ->minus_result;
a celý ten první switch by neměl být v controlleru ale v modelu, že?

Trochu mi ale není jasné to že "model by měl obsahovat výsledek, ne ho vracet". Mohl byste mi to prosím osvětlit?

Jinak mockrát děkuji.

radek o kterem pises bys naopak mel byt ve viewu (samozrejme rovnou vypis, zadne prirazovani uz neni potreba), z controlleru do viewu posles promenne, view pak doplni vsechno ostatní, to že vypisujes do promenne uz rovnou cely text "Výsledek: x" se okradas o moznost udelat view napriklad multijazycny (at uz budes nacitat pro kazdy jazyk jiny view nebo do viewu nacitat podle jazyku jine stringy, ktere budes nekde kumulovat)

ackoliv jde o to ja to mysli vedouci, me to totiz prijde spravne tak, ze controller zavola potrebne veci v modelu, posle viewu a ten uz pouze zobrazuje, texty celkove pak by meli byt na viewu, controller by mel posilat teoreticky surova data...

ale nepovazuji se za zadneho odbornika, treba me vedouci zas vyvede z omylu :) (to on umi :D )

Vzdycky jde o pohled na vec - budto se na kalkulator muzes divat jako na request-response bezstavovou vec a pak bude controller opravdu volat model a predavat do view navratovou hodnotu - pak ale defacto programujes ve funkcionalnim stylu - vracis a nic nedrzis.

Ale da se na to ale taky divat jako na stavovou entitu - a pak by to vypadalo takhle:

Kód:

<?php

/**

	Very simple model2 example

	One model, one controller, multiple views, multiple actions.
	Lot of getters, setters, duplication and other bad things...
	
	Some "code-golf" included

*/

//Model
class Calculator{
	private $a, $b, $result, $lastAction;
	
	function getA(){ 
		return $this->a;
	}
	
	function setA($newA){
		$this->a = (int) $newA;
	}

	function getB(){
		return $this->b;
	}
	
	function setB($newB){
		$this->b = (int) $newB;
	}

	function getResult(){
		return $this->result;
	}
	
	function getLastAction(){
		return $this->lastAction;
	}
	
	function add(){
		$this->result = $this->a + $this->b;
		$this->lastAction = __FUNCTION__;
	}
	
	function subtract(){
		$this->result = $this->a - $this->b;
		$this->lastAction = __FUNCTION__;
	}
}

//View
class CalculatorView{
	//model
	private $calculator;

	function __construct($injectedCalculator){
		$this->calculator = $injectedCalculator;
	}
	
	function render(){
		printf(
			'
				<style>
					body{font-family: sans-serif; font-size: 0.8em}
					a{display: inline-block; background: #666; color: #fff; text-decoration: none; padding: 0.5em 1em}
				</style>
				
				<h1>Current state</h1>
				<p>
					%s(%d, %d) = %d
				</p>
				
				<p>
					<a href="?change">Change numbers...</a>
					<a href="?invalidateSession">Invalidate session</a> (will initialize new calculator with random numbers)
				</p>
				
				<a href="?add">Add</a>
				<a href="?subtract">Subtract</a>
			',
			//can be localized here
			$this->calculator->getLastAction(),
			$this->calculator->getA(),
			$this->calculator->getB(),
			$this->calculator->getResult()
		);
	}
}

//Another view
class CalculatorForm{
	//model
	private $calculator;

	function __construct($injectedCalculator){
		$this->calculator = $injectedCalculator;
	}
	
	function render(){
		printf(
			'
				<h1>Change numbers</h1>
			
				<form action="?update" method="post">
					<table>
						<tr>
							<th>A:</th>
							<td><input name="a" value="%d" /></td>
						</tr>
						<tr>
							<th>B:</th>
							<td><input name="b" value="%d" /></td>
						</tr>
						<tr>
							<th></th>
							<td><input type="submit" /></td>
						</tr>
					</table>
				</form>
			',
			$this->calculator->getA(),
			$this->calculator->getB()
		);
	}
}

class CalculatorController{
	//model
	private $calculator;

	function __construct($injectedCalculator){
		$this->calculator = $injectedCalculator;
	}

	function show(){
		$view = new CalculatorView($this->calculator);
		$view->render();
	}
	
	function add(){
		$this->calculator->add();
		$this->show();
	}

	function subtract(){
		$this->calculator->subtract();
		$this->show();
	}

	function change(){
		$view = new CalculatorForm($this->calculator);
		$view->render();
	}
	
	function update(){
		$this->calculator->setA($_POST['a']);
		$this->calculator->setB($_POST['b']);
		
		//redirect after post
		header('Location: ' . $_SERVER['SCRIPT_NAME']);
	}
	
	function invalidateSession(){
		session_destroy();
		
		//redirect after post
		header('Location: ' . $_SERVER['SCRIPT_NAME']);		
	}
}

//init
session_start();

//try to load, otherwise init and store it for later usage
if ( ! @$model = & $_SESSION['calc'])
{
	$model = new Calculator() ;
	$model->setA(rand(10, 20));
	$model->setB(rand(0, 9));
	$model->add();
}
	
//default action
if ( ! $action = key($_GET))
	$action = 'show';

$controller = new CalculatorController($model);
$controller->$action();

K nahlednuti to bezi tady: http://codepad.viper-7.com/pLQU5X


Jak vidite, je to s prominutim sracka, 200 radku, 4 tridy, spousta getteru a setteru, atd. - ale tohle se povazuje za Model2 architekturu - controller zna model, na zaklade requestu zavola nejakou metodu a vybere view a nasledne ho jenom zobrazi.

mohlo by se zdat, ze v kohane to funguje jinak - ze controller ziskava model, ale to je zkreslene, modelem je totiz v tomto pripade ORM - to, ze si vytahnu radek a ze nemam samostatny controller pro ten radek, to uz je vec druha.

a treti vec je, ze model je v kohane globalni - ORM::factory("xxx") - fuj, fuj, fuj :)

cele se to da dost podstatne zjednodusit, ale o tom zase nekdy priste ;)