CakePHP Ticket Component

Question

I have the following code that allows easy creation, pulling, and deletion of Tickets in a CakePHP application.

App::uses('Component', 'Controller');
App::uses('Ticket', 'Model');
App::import(array('Security'));
class TicketComponent extends Component
{
 public $name = 'Ticket';
 // Create a new ticket by providing the data to be stored in the ticket. 
 function set($info = null) 
 { 
 $this->garbage(); 
 if ($info) 
 { 
 $ticketObj = new Ticket(); 
 $data['Ticket']['hash'] = md5(time()); 
 $data['Ticket']['data'] = $info; 
 if ($ticketObj->save($data)) 
 { 
 return $data['Ticket']['hash']; 
 } 
 } 
 return false; 
 } 
 // Return the value stored or false if the ticket can not be found. 
 function get($ticket = null) 
 { 
 $this->garbage(); 
 if ($ticket) 
 { 
 $ticketObj = new Ticket(); 
 $data = $ticketObj->findByHash($ticket); 
 if (is_array($data) && is_array($data['Ticket'])) 
 { 
 // optionally auto-delete the ticket -> this->del($ticket); 
 return $data['Ticket']['data']; 
 } 
 } 
 return false; 
 } 
 // Delete a used ticket 
 function del($ticket = null) 
 { 
 $this->garbage(); 
 if ($ticket) 
 { 
 $ticketObj = new Ticket(); 
 $data = $ticketObj->findByHash($ticket); 
 if ( is_array($data) && is_array($data['Ticket']) ) 
 { 
 return $data = $ticketObj->delete($data['Ticket']['id']); 
 } 
 } 
 return false; 
 } 
 // Remove old tickets 
 function garbage() 
 { 
 $deadline = date('Y-m-d H:i:s', time() - (24 * 60 * 60)); // keep tickets for 24h. 
 $ticketObj = new Ticket(); 
 $data = $ticketObj->query('DELETE from tickets WHERE created < \''.$deadline.'\''); 
 } 
}

It feels a little old now (not sure how old the code is) but wondered if certain things could be improved and if there was any parts that were considered 'bad code'.

Some observations of my own:

• Do I need to keep creating the $ticketObj? And is using new Ticket() correct for CakePHP conventions?
• The garbage function runs when anything ticket related is called, and makes sure that expired tickets cannot be used... But requires that it be called at the beginning of all other functions. Could this be better? Not necessarily a CRON job, but more code efficient.

Answer 1

In order to call each time the garbage function just create a

public function beforeFilter() {
 parent::beforeFilter();
 $this->garbage();

About the new Ticket() i think it would be okay to change the

$ticketObj->..

to

$this->Ticket->..

Also i think setting your function name's as set is a bad idea because they are built in functions in cakephp