Issues

ZF-3428: Zend_Uri_Http should not need a factory

Issue Type: Improvement Created: 2008-06-10T05:43:57.000+0000 Last Updated: 2008-09-02T10:39:06.000+0000 Status: Resolved Fix version(s): - 1.6.0 (02/Sep/08)

Reporter: Robin Skoglund (robinsk) Assignee: Ben Scholzen (dasprid) Tags: - Zend_Uri

Related issues: Attachments:

Description

Problem To create URIs today you have to go via the factory in Zend_Uri, because the constructor in Zend_Uri_Http is protected. This creates an additional overhead that simply is not needed. The constructor in Zend_Uri_Http might as well be public, since most of us already know that a HTTP URI is what we want to create.

Why make the constructor public?

Creating it via a factory gives nothing but overhead when you know that Zend_Uri_Http is what you want

Auto-completion in IDEs. Zend_Uri_Http has a lot of methods, and looking up the API is a tedious task. To get auto-completion (PDT) you currently have to 1) create the URI using a factory in a separate method then the one you're in, then 2) store it as a property in the class, 3) use the property's docblock to indicate that it is a Zend_Uri_http, and 4) use this property in other methods.

What to look out for when making the constructor public Currently there is no check on $scheme in the constructor, so the following code must/should be used instead of $this->_scheme = $scheme:

<pre class="highlight">
switch ($scheme) {
    case 'http':
    case 'https':
        $this->_scheme = $scheme;
        break;
    default:
        throw new Zend_Uri_Exception("Scheme \"$scheme\" is not supported");
}

Optionally there could be a factory method in Zend_Uri_Http that does the same as Zend_Uri::factory, only the new factory method may only create Zend_Uri_Http (obviously). This way the constructor may be kept protected and unmodified. This approach may be used if you don't know whether the URIs you create are 'http' or 'https'.

Consequences

Easier to use API

More friendly to IDEs

Does not break backwards compatibility

Ben Scholzen has agreed to fix this if approved.

Comments

Posted by Ben Scholzen (dasprid) on 2008-06-10T07:40:24.000+0000

Actually, the factory pattern is only there to make the creation of objects easier (by finding the right class to use), but not to forbid the creation of those specific classes. So another factory method would be wrong in that place, we have to make the constructor method public, and change it's behaviour, imho.

Posted by Ben Scholzen (dasprid) on 2008-06-10T07:44:37.000+0000

Well, so I suggest that the constructor can either behave like before (with two parameters), or with a single parameter (the url itself). Any suggestions on the variable naming then?

Posted by Robin Skoglund (robinsk) on 2008-06-10T07:51:35.000+0000

Yes. Also, since 'http' and 'https' don't make different classes, a single 'factory' method in Zend_Uri_Http just doesn't make sense (pattern-wise).

Another issue is that if the constructor is made public, you still would have to know if you're creating a 'http' or 'https' URI when constructing. This could be solved by creating another factory method called e.g. Zend_Uri_Http::fromString($uri), which would sniff out $scheme and $schemeSpecific to create the URI accordingly, and throw an exception if scheme is not 'http' or 'https'. This would be 100% backwards compatible.

Posted by Ben Scholzen (dasprid) on 2008-06-10T07:55:40.000+0000

I like the idea with the fromString() method. Sounds even better than making the constructor public and doesn't require to change current code. If approved I would make the neccesary changes.

Posted by Ben Scholzen (dasprid) on 2008-06-10T09:24:38.000+0000

Fixed with the Zend_Uri_Http::fromString() solution. By this, also rewrote the code to match new coding standard.

Posted by Ben Scholzen (dasprid) on 2008-06-10T09:42:26.000+0000

Comitted with revision 9655

Posted by Ben Scholzen (dasprid) on 2008-06-10T10:03:13.000+0000

Correction, fix revision is 9656

Posted by Wil Sinclair (wil) on 2008-09-02T10:39:06.000+0000

Updating for the 1.6.0 release.

Have you found an issue?

See the Overview section for more details.

Copyright

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

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

Contacts