Re: [PEPr] +1 for Web Services::Services_ShortURL

On May 20, 2009, at 2:40 PM, Michael Gauthier wrote:

1.) cURL extension is no required or optional in package.xml but HTTP_Request2 is created with cURL by default. 2.) Why explicitly use cURL by default? Why not let HTTP_Request2 decide that one?

There was a bug surfaced in HTTP_Request2 in the sockets driver so I forced cURL in the default setup. I think this has been fixed, but I'm not sure if it has been released. I'll go back and look. If it's fixed I can remove all of that.

3.) The accept() method is confusing to me. Why not setRequest(). Also, you could use type hinting here rather than checking the parameter type manually. The only reasons I can think of for doing it this way are if you are implementing a generic interface but that does not seem to be the case.

It's a simple implementation of the adapter/accept pattern. I think HTTP_Request2 does something similar.

4.) License tag links shouldn't use tinyurl because links could disappear in the unlikely even that tinyurl disappears.

You don't like irony do you? ;)

5.) That's all I've got. Looks great.

Thanks! --Joe