ZF-8107: Zend_Http_Clinet getAdapter() is not needed.
Description
I mentioned #ZF-8027 http://framework.zend.com/issues/browse/ZF-8027
trunk(r18637)'s "getAdapter()" is not needed , and remove, please. (Over API may cause bug..)
I mentioned #ZF-8027 http://framework.zend.com/issues/browse/ZF-8027
trunk(r18637)'s "getAdapter()" is not needed , and remove, please. (Over API may cause bug..)
Comments
Posted by Satoru Yoshida (satoruyoshida) on 2009-10-20T18:34:37.000+0000
Hi, Kazusuke Sasezaki.
I think getAdapter would bring us some benefit. Do you have any reason why the function should be removed?
I will be happy if you would tell us anything until next minor release.
Because once the function would be released, we must wait next major release for removing funtions.
Posted by Kazusuke Sasezaki (sasezaki) on 2009-10-21T07:37:00.000+0000
Hi, Satoru!
I have 3 reasons at least.
And Unfortunatoly, getter's doc-comment is same as mutator.
Posted by Michael Stillwell (mjs) on 2009-10-22T02:25:55.000+0000
As I mentioned in ZF-8027 getAdapter() is useful when debugging. In my case something was setting the adapter to something unexpected, but without a getAdapter() there was no way to even determine the class/type of the adapter.
getAdapter() is also probably useful when using some of the test adapters. For example if you need to dynamically modify the next "response" that the adapter will give.
Posted by Kazusuke Sasezaki (sasezaki) on 2009-10-22T07:20:33.000+0000
Hi, Michael. When debugging. Is not there a way to use other extended-class?
Posted by Kazusuke Sasezaki (sasezaki) on 2009-10-22T07:31:53.000+0000
Anyway, current getAdapter() is not correct implemented code against its doc-comment.
And, this changeset was Committed without testcode. http://framework.zend.com/code/changelog/…
Posted by Michael Stillwell (mjs) on 2009-10-23T02:59:42.000+0000
I'm not completely sure, but I don't think the getAdapter() should be in the business of creating an adapter if it doesn't already exist. (One reason: if this is the case, then there's no way to tell whether the adapter is already set.) I don't think it's necessary to check the type of the adapter since setAdapter() already does this. (i.e. the return type of getAdapter() will never be a lie, because there's no way to set the adapter to anything other than something that implements Zend_Http_Client_Adapter_Interface.)
However, I do think there is a problem arising from the patch in that if getAdapter() exists, all direct reads of $this->adapter (for example, in Zend_Http_Client's request() method) should be converted into reads of getAdapter() instead.
Regarding reason (1) above, I think the problem that calling methods on the adapter directly can lead to nonsensical results is more a consequence of the somewhat strange way the client uses the adapter rather than a problem that arises because getAdapter() exists.
I don't really understand (2).
Regarding (3), I think there probably should be a getConfig() method as well.
Posted by Michael Stillwell (mjs) on 2009-10-23T03:12:11.000+0000
Oh, and regarding why I can't extend Zend_Http_Client myself, and add a getAdapter() method to that: in my case the client is being built by a call to a factory object that I don't have any control over.
Posted by Kazusuke Sasezaki (sasezaki) on 2009-10-23T06:19:33.000+0000
Michael Stillwell, thank you very much for your comment.
By the enough reason in above comment, I satisfied myself. So, I want to close this issue.
Posted by Dolf Schimmel (Freeaqingme) (freak) on 2009-10-23T07:08:38.000+0000
Closing per request.