1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

Why have a new variable for each model function?

Discussion in 'XenForo Development Discussions' started by Marcus, Oct 26, 2012.

  1. Marcus

    Marcus Well-Known Member

    Most code looks like this:
    PHP:
      $noteModel =  $this->_getNoteModel() ;
      
    $notes $noteModel->getLatestNotes();
    instead of just $notes = $this-> getLatestNotes();

    Is it more elegant? Or does it activate internal php caching?
     
  2. Chris D

    Chris D XenForo Developer Staff Member

    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 and tenants like this.
  3. Marcus

    Marcus Well-Known Member

    thanks! Is this also cached:

    $notes = $this-> getLatestNotes();
     
  4. Chris D

    Chris D XenForo Developer Staff Member

    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.
     
  5. Marcus

    Marcus Well-Known Member

    So to sum up, this is usually cached:
    PHP:
     $noteModel $this->_getNoteModel() ;
      
    $notes $noteModel->getLatestNotes();
    ... and this is not always cached:
    PHP:
    $notes $this-> getLatestNotes();
    Is that correct?
     
  6. Chris D

    Chris D XenForo Developer Staff Member

    $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 likes this.
  7. Marcus

    Marcus Well-Known Member

    You are right with my typo ! :) I took the code from the scratchpad demo where Kier demonstrates the AJAX requests. I meant
    PHP:
    $notes $this->_getNoteModel()->getLatestNotes($maxNotes);
    Is that cached, too?

    At most places within xenforo it would be written as
    PHP:
     $noteModel $this->_getNoteModel() ;
      
    $notes $noteModel->getLatestNotes();
    And I wonder why there is always a new $.....Model variable introduced in each controller.
     
  8. Chris D

    Chris D XenForo Developer Staff Member

    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.

    Typically there is a separate model for each controller.
     
    Marcus likes this.
  9. Marcus

    Marcus Well-Known Member

    At Xenforo_ControllerPublic_Thread::actionIndex there are two models introduced:
    PHP:
     $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?
     
  10. tenants

    tenants Well-Known Member

    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:
    PHP:
    $threads $this->_getThreadModel()->getAllThreads();
    or this:
    PHP:
    $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
    PHP:
    $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
     
    Marcus likes this.
  11. Luke F

    Luke F Well-Known Member

    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.

    Code:
    /** @var XenForo_Model_User */
    $userModel = $this->getModelFromCache('XenForo_Model_User');
     
    Marcus likes this.

Share This Page