Yii Framework Forum: Can't Extend Yii Class Because Of Private Attributes - Yii Framework Forum

Jump to content

Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

Can't Extend Yii Class Because Of Private Attributes Rate Topic: -----

#1 User is offline   radzserg 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 15
  • Joined: 13-June 12

Posted 12 February 2013 - 06:45 AM

I've already created a topic http://www.yiiframew...__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.
0

#2 User is offline   redguy 

  • Master Member
  • PipPipPipPip
  • Yii
  • Group: Members
  • Posts: 814
  • Joined: 02-July 10
  • Location:Central Poland

Posted 12 February 2013 - 07:25 AM

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.
red
0

#3 User is offline   radzserg 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 15
  • Joined: 13-June 12

Posted 12 February 2013 - 02:45 PM

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
0

#4 User is offline   radzserg 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 15
  • Joined: 13-June 12

Posted 12 February 2013 - 02:48 PM

View Postredguy, on 12 February 2013 - 07:25 AM, said:

To access data returned by default implementation you can use parent::getRestParams() and then process this data before returning.


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

public function getRestParams() {
    return parent::getRestParams() 
}


very nice ;)
0

#5 User is offline   redguy 

  • Master Member
  • PipPipPipPip
  • Yii
  • Group: Members
  • Posts: 814
  • Joined: 02-July 10
  • Location:Central Poland

Posted 12 February 2013 - 02:58 PM

View Postradzserg, on 12 February 2013 - 02:45 PM, said:

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

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

radzserg said:

Right so now my method that will return restParams in child classes will looks like this
public function getRestParams() {
    return parent::getRestParams() 
}


very nice

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...
red
0

#6 User is offline   radzserg 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 15
  • Joined: 13-June 12

Posted 12 February 2013 - 04:05 PM

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(...)
0

#7 User is offline   redguy 

  • Master Member
  • PipPipPipPip
  • Yii
  • Group: Members
  • Posts: 814
  • Joined: 02-July 10
  • Location:Central Poland

Posted 13 February 2013 - 04:02 AM

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.
red
0

#8 User is offline   redguy 

  • Master Member
  • PipPipPipPip
  • Yii
  • Group: Members
  • Posts: 814
  • Joined: 02-July 10
  • Location:Central Poland

Posted 13 February 2013 - 04:24 AM

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

#9 User is offline   radzserg 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 15
  • Joined: 13-June 12

Posted 13 February 2013 - 04:33 AM

View Postredguy, on 13 February 2013 - 04:02 AM, said:

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
}



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.
0

#10 User is offline   redguy 

  • Master Member
  • PipPipPipPip
  • Yii
  • Group: Members
  • Posts: 814
  • Joined: 02-July 10
  • Location:Central Poland

Posted 13 February 2013 - 05:00 AM

View Postradzserg, on 13 February 2013 - 04:33 AM, said:

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

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

View Postradzserg, on 13 February 2013 - 04:33 AM, said:

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.

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...
red
0

#11 User is offline   radzserg 

  • Newbie
  • Yii
  • Group: Members
  • Posts: 15
  • Joined: 13-June 12

Posted 14 February 2013 - 03:30 AM

Quote

Setting proper visibility scope and providing dedicated interfaces to interact with classes and its members is crucial in OOP

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.
http://stackoverflow...ember-variables

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.
0

#12 User is offline   redguy 

  • Master Member
  • PipPipPipPip
  • Yii
  • Group: Members
  • Posts: 814
  • Joined: 02-July 10
  • Location:Central Poland

Posted 14 February 2013 - 04:14 AM

...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.
red
0

Share this topic:


Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

1 User(s) are reading this topic
0 members, 1 guests, 0 anonymous users