Gdy przyjrzysz się bliżej kontrolerom stworzonym przy pomocy narzędzia ‘bake’ (lub takim, jakie powstają po wykonaniu tutoriala) możesz stwierdzić, że łamią one koncepcję DRY. Jesteś w stanie powiedzieć, które z dwóch metod są niemal identyczne? Jeśli nie – sprawdź, które dwa widoki są niemal identyczne. Ok, może spaliłem swoją zagadkę, bo w tytule tego postu jest wyraźna sugestia o czym będę mówił. Zgadza się.

Najpierw rzućmy okiem na widoki (tu w przykładzie dla Modelu Thing, który ma jedno pole – name):

// app/views/things/add.ctp
<div class="things form">
<?php echo $this->Form->create('Thing');?>
   <fieldset>
      <legend>
         <?php printf(__('Add %s', true), __('Thing', true)); ?>
      </legend>
   <?php
      echo $this->Form->input('name');
   ?>
      </fieldset>
<?php echo $this->Form->end(__('Submit', true));?>
</div>
<div class="actions">
   <h3><?php __('Actions'); ?></h3>
   <ul>
      <li>
         <?php echo $this->Html->link(
            sprintf(
               __('List %s', true),
               __('Things', true)
            ),
            array('action' => 'index')
         );?>
      </li>
   </ul>
</div>

// app/views/things/edit.ctp
<div class="things form">
<?php echo $this->Form->create('Thing');?>
   <fieldset>
      <legend>
         <?php printf(__('Edit %s', true), __('Thing', true)); ?>
      </legend>
   <?php
      echo $this->Form->input('id');
      echo $this->Form->input('name');
   ?>
   </fieldset>
<?php echo $this->Form->end(__('Submit', true));?>
</div>
<div class="actions">
   <h3><?php __('Actions'); ?></h3>
   <ul>
      <li>
         <?php echo $this->Html->link(
            __('Delete', true),
            array('action' => 'delete', $this->Form->value('Thing.id')),
            null,
            sprintf(
               __('Are you sure you want to delete # %s?', true),
               $this->Form->value('Thing.id')
            )
         ); ?>
      </li>
      <li>
         <?php echo $this->Html->link(
            sprintf(__('List %s', true), __('Things', true)),
            array('action' => 'index')
         );?>
      </li>
</ul>
</div>

Różnią się dokładnie w trzech linijkach (na 16 łącznie). Jeśli zmienisz coś w jednym (na przykład dodasz pole) to będziesz musiał poprawić formularz w dwóch plikach. Czysta głupota.
Przyjrzyjmy się jeszcze metodom:

// app/controllers/things_controller.ctp
function add() {
   if (!empty($this->data)) {
      $this->Thing->create();
      if ($this->Thing->save($this->data)) {
         $this->Session->setFlash(__('The thing has been saved', true));
         $this->redirect(array('action' => 'index'));
      } else {
         $this->Session->setFlash(
            __('The thing could not be saved. Please, try again.', true)
         );
      }
   }
}

function edit($id = null) {
   if (!$id && empty($this->data)) {
      $this->Session->setFlash(__('Invalid thing', true));
      $this->redirect(array('action' => 'index'));
   }
   if (!empty($this->data)) {
      if ($this->Thing->save($this->data)) {
         $this->Session->setFlash(__('The thing has been saved', true));
         $this->redirect(array('action' => 'index'));
      } else {
         $this->Session->setFlash(
            __('The thing could not be saved. Please, try again.', true)
         );
      }
   }
   if (empty($this->data)) {
      $this->data = $this->Thing->read(null, $id);
   }
}

Praktycznie całe ciało metody add() można by wkleić w miejsce drugiego if’a w metodzie edit() i niewiele by się zmieniło, prawda?

Kilka tricków pozwoli nam naprawić to marnotrawstwo. W zasadzie najważniejsza kwestia jest następująca – jak działa metoda $this->Thing->create()? Wymusi dodanie nowego elementu (dlatego, że metoda save() działa jako create lub update w zależności od stanu modelu). Ale nie przekazując żadnych parametrów do tej metody tak naprawdę sprowadzi się do wyczyszczenia $this->data i $this->id (this oznacza w tym przypadku model). W przypadku metody add() nie ma możliwości, żeby jakieś śmieci były w $this->Thing->data, więc możemy spokojnie usunąć tą linijkę – dalej będzie działać tak samo (czy na pewno? – spoiler).
Teraz całkowicie ciało metody add() mogłoby zawrzeć się w metodzie edit(). Dlatego trzeba ją usunąć (razem z jej widokiem). A wszystkie linki, które prowadziły do /things/add niech teraz prowadzą do /things/edit.
Warto sprawdzić, co się teraz dzieje?

W zależności od wersji – 1.2 się wywali, 1.3 pokaże informacje “Invalid thing”. Teraz skupię się na wersji 1.3 – bo na niej sprawdzam na bieżąco efekty pisząc ten wpis. (dla 1.2 jeśli dobrze pamiętam wystarczy wstępnie zmienić sygnaturę metody edit($id) na edit($id=null), dalsze kroki będą analogiczne, choć kod w szczegółach może się różnić)

Od teraz możemy założyć, że jeśli w url’u nie ma podanego id – to znaczy, że chcemy dodać nowy element, zatem tego if’a możemy usunąć:

// app/controllers/things_controller.php
		if (!$id && empty($this->data)) {
			$this->Session->setFlash(__('Invalid thing', true));
			$this->redirect(array('action' => 'index'));
		}

Możemy spokojnie teraz dodawać nowe elementy przez formularz- wszystko działa.
Warto jeszcze dopieścić widok – bez sensu jest nagłówek “edit” przy dodawaniu elementu, oraz przycisk “delete”,
fragment z legend poprawiamy na taki (dodałem wcięcia dla przejrzystości):

// app/views/things/edit.ctp
 		<legend>
 		   <?php printf(
 		      (!is_null($this->data["Thing"]["id"])?__('Edit %s', true): __('Add %s', true)),
 		      __('Thing', true));
 		   ?>
 		 </legend>

a pierwszy element li otaczamy if’ami tak:

// app/views/things/edit.ctp
<?php if(!is_null($this->data["Thing"]["id"])): ?>
   <li>
      <?php echo $this->Html->link(
         __('Delete', true),
         array('action' => 'delete', $this->Form->value('Thing.id')),
         null,
         sprintf(
            __('Are you sure you want to delete # %s?', true),
            $this->Form->value('Thing.id')
         )
      ); ?>
   </li>
<?php endif; ?>

I tyle.

Usunięcie $this->Thing->create() nie takie bezpieczne?
Napisałem, że po usunięciu tego elementu nic się złego nie stanie. Oczywiście jest to prawdą pod warunkiem, że wykonasz wszystkie czynności opisane w tym poscie. Jeśli tylko usuniesz $this->Thing->create() narażasz się na niebezpieczeństwo, że ktoś wyśle spreparowanego post’a pod ten adres. Jeśli poda w nim pole o nazwie data[Thing][id] – będzie mógł zmienić dane dla elementu o danym id. Jeśli masz sytuację taką, że jedna grupa może dodawać elementy, a inna je edytować i polegasz tylko na acl’ach (dana grupa ma dostęp do edit(), a kolejna do add()) – to w ten sposób możesz stworzyć lukę. Jednak ta sytuacja jest dość rzadka i to jest raczej temat na inny wpis, więc jeśli interesuje Cię rowiązanie tego problemu – zachęcam do komentowania.