Can't Extend Yii Class Because Of Private Attributes

I’ve already created a topic http://www.yiiframework.com/forum/index.php/topic/39867-how-to-get-json-decoded-postdelete-data/page__p__190948__fromsearch__1 where described problem with CHttpRequest and private $_restParams property. Shortly we can’t extend CHttpRequest::getRestParams because $_restParams is private and getRestParams() won’t work as you exptected in your child class.

But we have the same problem in the whole framework. In order to override 1 method that changes some private property you need to copy paste other that return this property.

For instance if you want to override CErrorHandler::handleException that changes $this->_error = … you need to copy paste getError method in order to get correct _error.

If you want to use your own CController::setPageTitle then you need to copy getPageTitle, $_pageTitle is private.

So there’s a question for yii team:

Of course this is clear that you want to reduce visibility and in general this good. But it is very likely that such components as CErrorHandler CWebApplication CController and others could be overriden. So maybe you should not be so critical with protected properties.

those properties are private probably for a reason. When you extend such class you must use only provided interface to access such data (there will probably be getError/setError methods). To access data returned by default implementation you can use parent::getRestParams() and then process this data before returning.

on the other hand wwhen you want totally rewrite default mechanizms you must provide your own attributes to store your data.

I don’t wanna totally rewrite anything I want to extend base classes, one or two methods that’s it! This is the key.

As I mentioned I can’t specify setPageTitle without updating or copy pasting getPageTitle for CController.

I can’t extend CErrorHandler::handleException or CErrorHandler::handleError without copypasting getError.

And the main discrepancy - take a look at handleException handleError they are protected. This means that it’s expected that this method could be overriden in child classes. But they changes private properties. And as I mentioned this requires copy pasting or additional functionality

Right so now my method that will return restParams in child classes will looks like this




public function getRestParams() {

    return parent::getRestParams() 

}



very nice ;)

yes, you can:




public function setPageTitle($title) {

  //do whatever is needed with $title

  parent::setPageTitle( $title . ' - My Site' );

}



there is no need to modify getPageTitle unless you have to process title somehow…

no, if you do not change the way method work you do not have to overload it in child classes… it is rather for cases like this:




public function getRestParams() {

    $params = parent::getRestParams();

    foreach( $params as $name=>$value ) {

        //do something with params

    }

    return $params;

}



those are basics of OOP…

Ok man, yes this will work with setPageTitle. Bad example.

But what about CHttpRequest there’s no ability to set $_restParams. Take a look into this class.

What about CErrorHandler there’s no ability to set $_error therefore you can’t override yours handleException handleError and then use $this->setError(…)

in CErrorHandler method handleException sets private _error attribute - yes that is true. So if you want to overload handleException you should do:




private $myError;


public function getError() {

   return $this->myError;

}

public function handleException() {

   ... handle exception and set $this->myError

}

public function handleError() {

   ... handle error and set $this->myError

}



because they are part of same interface… maybe there should be also protected ‘setError’ so you could overload only ‘handleException’ and use that setter, but the interface was designed this way. The problem is for sure not with private member attribute ‘_error’.

as for _restParams in CHttpRequest - this attribute is only for caching purpose. see body of getRestParams() function:




public function getRestParams()

{

    if($this->_restParams===null)

    {

        $result=array();

        if(function_exists('mb_parse_str'))

            mb_parse_str($this->getRawBody(), $result);

        else

            parse_str($this->getRawBody(), $result);

        $this->_restParams=$result;

    }


    return $this->_restParams;

}



so if you want to change something here - you mus override this method and provide your own caching member attribute. Unfortunately there are getPut and getDelete methods which are in fact implemented poorly, as they depend on this attribute (and they should not)… So again - in this case you should overload getRestParams and getDelete/getPut methods because they depend on same attribute.

about getDelete and getPut methods implementation I created a bug report: https://github.com/yiisoft/yii/issues/2106

That’s what I’m talking about - I can’t just extend handleException without additional changes. And in fact I’ll get private $_error in parent class and private $myError that’s very confusing. But in fact it’s ABSOLUTELY the same property. IMHO in this case it’s better to use the same name.

As for CHttpRequest - yes, I can’t extend getRestParams because getDelete and getPost won’t work.

So I have to copy-paste logic. And it’s not important would it be my new property(myError) or the property with the same name(_error). I still need to add methods like getError, getDelete, getPost that will do absolutely the same function and in fact are useless. So why should I do that.

The easiest and IMHO absolutely correct solution to make such properties protected. It’s assumed that you can and you will extend functionality related with this properties in your child classes.

and looking at comments under my issue report - it will be fixed in next release.

maybe it would be easiest but it is not the proper way. there are cases when attributes should stay private, and _restParams is one of such cases. Maybe _error could be protected or maybe rather there should be protected setError setter called in handleException/handleError. You may report that as a bug (but rather go with setError solution or it will be rejected by the Yii team). Setting proper visibility scope and providing dedicated interfaces to interact with classes and its members is crucial in OOP. setting everything public or protected is not OOP, but rather structural programming with methods assigned to structures…

Yes, this’s very true. And nobody tells that it’s bug or not.

But according to your thought we shouldn’t use protected properties at all. Why should we use it if we can specify getter/setter and in general it would be more flexible solution cause potentially we can add some logic inside these getter/setter. Am I right?

Actually this is a very tricky question. I found quite interesting thread about protected member variables on stackoverflow.

And there’s no ultimate solution for that all depends on what strategy/conception have you chose. And maybe you’re right. That’s why my question was initially addressed to yii team. I’d like to know their vision on that problem.

…and I am not orthodox about ‘private’ members and getters/setters interfaces :)

My point was only that setting all members accessible is also not a perfect solution. In your particular examples there are possible design flaws in Yii classes (one already reported with pending fix) that cause overriding methods much more complicated.