ZF-8669: Zend_File_Transfer: Replace constructor with factory method

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:

  1. $adapter - the name of the adapter, relative to the Zend_File_Transfer_Adapter namespace (i.e., 'http')
  2. $config - Either a PHP array, or Zend_Config instance, of configuration options.

The $config supports all of the options that the ZFT adapters support, and also two ZFT-specific options:

  1. adapter: the name of the adapter to use
  2. adapterNamespace: a user-defined namespace to use in place of the default 'Zend_File_Transfer_Adapter' namespace.

Side Note: I have read the proposal at… 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:…

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.


© 2006-2018 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.