Why have a new variable for each model function?

[Marcus]

Most code looks like this:

  $noteModel =  $this->_getNoteModel() ;
  $notes = $noteModel->getLatestNotes();

instead of just $notes = $this-> getLatestNotes();

Is it more elegant? Or does it activate internal php caching?

 

[Chris D]

If you look at the $this->_getNoteModel you will see that it calls another function to fetch a model from the cache. If the model has already been instantiated with XenForo_Model::create then it will not do it again as the response from that is cached.

 

[Marcus]

thanks! Is this also cached:

$notes = $this-> getLatestNotes();

 

[Chris D]

I don't know. It depends what it is and how it's been written.

Usually if it's just a query in the model, probably not.

 

[Marcus]

So to sum up, this is usually cached:

 $noteModel = $this->_getNoteModel() ;
  $notes = $noteModel->getLatestNotes();

... and this is not always cached:

$notes = $this-> getLatestNotes();

Is that correct?

 

[Chris D]

$noteModel is the variable which stores the cached model (which is cached when XenForo_Model::create('Model_Name') is instantiated).

$noteModel->getLatestNotes(); is probably a simple query to fetch the notes and actually probably not cached.

I don't really understand the $notes = $this-> getLatestNotes bit. It is syntactically incorrect (due to the space after $this->). I assumed you'd made a typo and that it was supposed to read $notes = $noteModel->getLatestNotes();

 

[Marcus]

You are right with my typo ! I took the code from the scratchpad demo where Kier demonstrates the AJAX requests. I meant

$notes = $this->_getNoteModel()->getLatestNotes($maxNotes);

Is that cached, too?

At most places within xenforo it would be written as

 $noteModel = $this->_getNoteModel() ;
  $notes = $noteModel->getLatestNotes();

And I wonder why there is always a new $.....Model variable introduced in each controller.

 

[Chris D]

$notes = $this->_getNoteModel()->getLatestNotes($maxNotes);

Is that cached, too?

Without looking at the code, in all likelihood it is only the action of loading the model that is cached ($this->_getNoteModel). There is no evidence to suggest anything else is cached, but I know that typically whenever a model is loaded it is loaded from the Model cache. If the Model cache for that particular Model (e.g. Notes, Notices, Threads or whatever) is empty at the time then it will be loaded into the cache so it isn't necessary to load the model again.

And I wonder why there is always a new $.....Model variable introduced in each controller.

Typically there is a separate model for each controller.

 

[Marcus]

At Xenforo_ControllerPublic_Thread::actionIndex there are two models introduced:

 $threadModel = $this->_getThreadModel();
  $postModel = $this->_getPostModel();
 
protected function _getForumModel()
{
return $this->getModelFromCache('XenForo_Model_Forum');
}
 
protected function _getThreadModel()
{
return $this->getModelFromCache('XenForo_Model_Thread');
}

I now understood how the Xenforo_Model::getModelFromCache function works. So there is no technical reason for using $threadModel->getAllThreads(); instead of $this->_getThreadModel()->getAllThreads(); ? Coming to my original question, is it more elegant to user the former method?

 

[tenants]

IMO I would say elegance is often in the eye of the beholder (it can be subjective), how you go about doing the same thing in one line or two lines is really up to you (some people prefer things spelt out for ease or reading, others prefer it shortened for ease of reading)

Doing this:

$threads = $this->_getThreadModel()->getAllThreads();

or this:

$threadModel = $this->_getThreadModel();
$threads = $threadModel->getAllThreads();

Really makes no difference,

they have both checked the cache (given that the helper checks the cache), but I would never substitute what looks good or clever for what is actually more efficient to process (in the above case, both are effectively the same)

I would use the 1st if I was only going to use the threadModel once in a function, and the second if I was to use it more than once (but that's just my preference for ease of reading, others may differ)

The below would not use a previously instantiated model

$threadModel = XenForo_Model::create('XenForo_Model_Thread');

Where possible avoid this and use the helper functions (or write your own) that call getModelFromCache

When it comes to efficiency, I would keep my eye on the query count and large loops, don't worry about an extra line. When it comes to elegancance, then it's how YOU read it back that's important

 

[Luke F]

I now understood how the Xenforo_Model::getModelFromCache function works. So there is no technical reason for using $threadModel->getAllThreads(); instead of $this->_getThreadModel()->getAllThreads(); ? Coming to my original question, is it more elegant to user the former method?

There's not really any reason whatsoever for either method, though the long way ($this->_getThreadModel()->getAllThreads())means more typing and less autocomplete. I would tend to use @var regardless, easier than having a bunch of helper methods to gain code completion. Possibly IDE-dependant though.

/** @var XenForo_Model_User */
$userModel = $this->getModelFromCache('XenForo_Model_User');