Issue Type: Improvement Created: 2009-12-30T06:21:55.000+0000 Last Updated: 2010-03-22T12:22:28.000+0000 Status: Resolved Fix version(s): - 1.10.3 (01/Apr/10)
Reporter: Ken Stanley (dohpaz) Assignee: Thomas Weidner (thomas) Tags: - Zend_File_Transfer
Related issues: - ZF-8668
Attachments: - ZF-8669.patch
Instead of the proposed patch in ZF-8668, I figured that ZFT would be better suited to have a factory() method for instantiating and returning a ZFT adapter (similar to Zend_Db::factory()). This proposed patch replaces the current constructor implementation with a Zend_Db::factory()-like factory method that accepts two parameters:
The $config supports all of the options that the ZFT adapters support, and also two ZFT-specific options:
Side Note: I have read the proposal at http://framework.zend.com/wiki/display/… and feel that this proposed patch still fits within the guidelines. The only difference is how the adapter is gotten: the proposal incorrectly implies that a direct instantiation would return an adapter, when it cannot. Thus, the factory method. The use cases would need to be updated to reflect this change. If there is more that I need to do to have this patch accpeted, then please advise.
Posted by Thomas Weidner (thomas) on 2009-12-31T05:34:52.000+0000
Closing as won't fix.
A factory method does not allow to have up- and download adapters attached. This would negotate the futural benefit of this component.
Posted by Ken Stanley (dohpaz) on 2009-12-31T06:44:39.000+0000
How do you figure? The whole point of the factory is to instantiate adapters. The entire concept was borrowed from Zend_Db, which has many adapters. For example, currently with the Zend_File_Transfer_Adapter_Http, you simply call Zend_File_Tranfer::factory('http', $http_adapter_options_array) to get the instance of the adapter. Similarly, when there are other adapters (such as the upload and download that you mention), you would call Zend_File_Transfer::factory('upload', $upload_adapter_options_array) and Zend_File_Transfer::factory('download', $upload_adapter_options_array).
How does this negate the "futural" benefit of this component? As it stands, the current implementation of Zend_File_Transfer::__construct() does not work. So really, this factory makes any "futural" benefit possible. :)
Posted by Ken Stanley (dohpaz) on 2009-12-31T06:49:35.000+0000
Additionally, with the extra 'adapterNamespace' parameter that may be passed to the options array, it allows extending existing adapters for full OO customization.
I really would appreciate you reconsidering your decision to deep-six this patch. Thank you.
Posted by Ken Stanley (dohpaz) on 2010-01-15T06:54:36.000+0000
I would appreciate a response, thank you. :)
Posted by Thomas Weidner (thomas) on 2010-03-19T13:21:31.000+0000
Closing as won't fix
No benefit for a switch from "new Object" to "factory()". It would even add a BC when the constructor is made protected and its no benefit to have 2 methods to initiate a object the same way.
Just because another component uses this pattern it does not mean that it's required for this component.
Posted by Ken Stanley (dohpaz) on 2010-03-21T15:12:26.000+0000
The problem is, you absolutely, positively CAN NOT return ANYTHING from a constructor. Therefore, the current implementation DOES NOT WORK in its current state. Therefore, the most logical solution is to convert the constructor into a factory method. So yes, because another component uses this pattern does mean it is required for this component.
Posted by Thomas Weidner (thomas) on 2010-03-21T15:40:31.000+0000
Following your conclusion means that Object Oriented Programming itself is completly useless as you think that the constructor of an object does not return anything.
In my understanding a constructor returns at last a new instance of the object which it has to create. Otherwise "new Object" would not work.
Posted by Ken Stanley (dohpaz) on 2010-03-21T18:12:34.000+0000
Have you not looked at your code? In the constructor you are attempting to return the adapter (not the constructor's class), which cannot be done. The changes that I propose would allow you to be able to instantiate one or more file transfer adapters, where as your code currently cannot, and does not. How does changing the constructor to a factory not fit the original intent of this class? It seems to me that you haven't even looked at the patch, and have simply made up your mind not to even give it any consideration.
Posted by Thomas Weidner (thomas) on 2010-03-22T12:04:56.000+0000
Closing as won't fix.
As mentioned before, your code breaks existing functionality. And planned addons are made impossible by your change.
I don't know from where you have the opinion that the constructur returns an adapter. It returns a new object of itself.
Posted by Ken Stanley (dohpaz) on 2010-03-22T12:22:28.000+0000
I have seen the error of my argument. Apparently, there were changes in 1.10.0 that were not noted in this ticket that changed the behavior from when this ticket was originally posted against 1.9.6. The constructor fiasco that I was speaking about can be seen here: http://framework.zend.com/svn/framework/…
My apologies. Seeing the updated 1.10.x version, I can see now what the solution that you spoke of is. My apologies for bothering you with this. :)
Have you found an issue?
See the Overview section for more details.