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

Best way to determine current controller

Discussion in 'XenForo Development Discussions' started by digitalpoint, Oct 25, 2011.

  1. digitalpoint

    digitalpoint Well-Known Member

    There are times I want to modify a Model, but only when on certain pages (for example adding a JOIN to the thread model, but only when viewing an individual thread).

    So I wanted to use my listener to only extend the class if we are on a certain page. Now I know I can use the $GLOBALS['fc'] object to do it, but it makes me cringe a bit doing it that way.

    Is there some static function for getting the current controller I'm missing?

    This works, but... blah... lol
    if ($GLOBALS['fc']->route()->getControllerName() === 'XenForo_ControllerPublic_Thread') ...
    cclaerhout and Luke F like this.
  2. Rigel Kentaurus

    Rigel Kentaurus Well-Known Member

    I am also wondering about this one ... just because I have valid scenarios in which I want to do this.

    For example, I extended the prepareThread() method on Thread_Model to modify the thread title, for some valid use case on my site. Thing is, I only want to do that on the ControllerPublic_Thread, not everytime... figuring out the controller would be ideal .. but sadly, no way of doing it.

    Recommendation for the devs: Have the current controller stored in a singleton object somewhere, that I could access with XenForo_Application::getController()

    This is not such an antipattern, we do that all the time in Java with thread locals. On the servlets world, one didn't have access to the request-response, jump some years to JSF, now I have a thread local with access to the request (and FacesContext, and some other things that pretty much equal the controller) from everywhere ...

    Then again, this is how I am doing that today ...

    1) I extend the controller, say, extend ControllerPublicThread on actionIndex()
    2) Inside actionIndex, I setup a flag (it can be global, but I created my own namespace (another class) for flags), so, pretty much...

    public function actionIndex()
        SomeClass::myFlag = true;
        return parent::actionIndex();
    3) Inside the model, I check for the flag. If it has been set, it means it is coming from the controller I am interested in

    public function someOverridenModelMethod()
        if (SomeClass::myFlag)
            // do something
  3. Kier

    Kier XenForo Developer Staff Member

    I've added XenForo_Application::getController(), which will return the current controller object in use (if any) but I'll wait for Mike to review it for evilness before I can tell you that it will definitely be in the next release.
  4. Kier

    Kier XenForo Developer Staff Member

    Following a discussion with Mike, we have decided not to include this code in XenForo. It can really only lead to gruesome abuses of MVC, and there are lots of other methods for having controllers set specific flags, such as the XenForo_Controller::_preDispatch() method, which could be extended using the load_class_controller code event.

    I do understand that our controllers are not as open as they could be right now, especially with regard to setting things like $fetchOptions for $model->getThings() queries, but we have some plans to remedy those deficiencies in the future.
  5. Mike

    Mike XenForo Developer Staff Member

    I disagree with this change strongly. The model should not depend on the controller directly. The controller should be telling the model what it wants. While not 100% ideal, the flag system is reasonable as it doesn't create a direct dependency -- it's just a variable.

    With the flag system, you can extend the controller and set it in pre-dispatch or in a particular action. You could even do it in the front controller.

    An alternative, depending on what you're trying to manipulate, is to manipulate the data returned for the view. Again, this can be done with extending the action or in post-dispatch, or even in the front controller.
    marioman, Onimua, Jeremy and 2 others like this.
  6. digitalpoint

    digitalpoint Well-Known Member

    Yeah, I totally agree with keeping model so they don't rely on there being a controller. From a design standpoint, flagging from the controller is better, but in my particular "real-world" case, I'll probably just stick with what I'm doing now (accessing the current controller via $GLOBALS['fc']), and just stick an @ on the front of it so the model still works if there is no controller.

    My reasoning is this... It's for something for internal use so I need efficiency over cleanliness, and I'm already getting worried about how many PHP included files are going to be on every page view for all custom addons I need to build for my site. As it is now, it's already looking like there is going to be 500-700 PHP files included for every page view because of all the possible class extensions. Not anywhere remotely done with all the custom stuff I need to build, and I'm already thinking about maybe somehow making a listener compiler. Like if I have 200 different LoadClassController listeners for various things, consolidate them all down to a single PHP file. Then again, maybe 500 included PHP files won't have any negative performance impact... will just have to test it when I get there.
  7. Kier

    Kier XenForo Developer Staff Member

    I'd certainly recommend merging commonly-used listeners into a single file.
  8. Ruven

    Ruven Well-Known Member

    BTW, the kind of situation going on in this thread is exactly why im using xenforo.
    Devs that listen to users, and then actually give thought, discussion and even testing to the ideas presented by the users.

    Novel concept.
    marioman, George and Kier like this.

Share This Page