Re: [PEPr] Call for votes on Web Services::Services_Webservice

weber@mayflower.de wrote:

weber@mayflower.de wrote:

1. As Sergio requested, it would have been better to have direct access to the source (.phps) instead of the .tgz... which I have now downloaded and found out there's only 1 class file in it, so it wouldn't have been too hard to simply add .phps link...

..

2. A lot of CS problem, especially in the lack of spaces around operators and so on.

Missing spaces about operators? That is not really a problem!

Right, not a huge problem, but must still be fixed to be accepted.

...

3. Extending the Webservices_Webservice class is not good and too restrictive. You should should use a decorator pattern that would accept a class name or an instance of it on which you run the reflection functions, and use the __call() PHP5 feature to pass the queries.

Extending Webservice is too restrictive? Decorator pattern? Passing instance? I better not start a discussion about that!

class a { } class b extend a { } then how do you expose class b as a webservice??? Extending Services_WebService is just plain bad design.

no it is not. it is not plain bad design like you said. it is the general way you do create webservices in other programming languages. That argument has never been very useful to the PEAR community. PHP is a language of its own, and comparing it to other languages for certain implementations doesn't necessarly work. Does it mean that because PHP doesn't support multiple inheritance that PHP is no good?

you do not create a webservice from many classes but from ONE class! Right, and still you didn't answer me on how to expose class b above as a webservice using your solution... and of course extending "class a" to be sub-class of Services_WebService is not one.

depending classes are considered as types within that class. So before we continue here i would propose you get some experience how that in general works. see what i really dont like and dont understand is how someone can come up with sentences like "that is plain bad design" without being informed enough. It's a bit presumptious for you to assume so.

I am not questioning the usefulness of the package, but it's too narrow implementation. If your package is not subject to reviews or suggestions, why bother submitting it to PEAR?

4. There is certainly a way to get some work with the upcoming XML_RPC2. Maybe the XML_RPC2_Server should be spun into some kind of XMLRPC profile to a generic WebService_Server package.

That is right.

We don't want to duplicate efforts to implement reflection, docblock parsing and so on for each different webservice protocol.

Who do you mean with "We" ? You are probably right. But like things are going here in Pear this wouldn`t lead to any result.

"we" = community of PEAR developer at large. It's not because there are heated discussions some times that they don't end up good. It would result in a better design (see recent discussion around XML_RPC2 and XML_RPC2_Value_* classes)

As is, I would probably vote a -1. But will hold my vote for now, to see how things are going first.

Is up to you!

5. For the "info" method, use standard HTML and not <div style="font-size: 36px"> type of things... An <h1> would have done just fine.

Yes, but I can also use <div>

Yes, you can, but then you are making it impossible to easily apply CSS. Standard tags exist, so use them. It makes life easier. the infopage does not exist to be changed from any user. it gives information about the webservice. so why discus this now? Because it is part of the review process... The best option would be of course to use some kind of template. The next best one is to use standard HTML tags so applying a CSS would be easier.. I can easily imagine that users of this package would want to integrate a call to the info page in their website well preserving their look and feel, or at least staying close to it. To do so, they could simply add a "<link>" tag in your package source. Also what about small screen devices that would be hard pressed to display "font-size: 36pt" or whatever you decide is the right choce. Using standard tags is the most effective way to do things, because their are *standard*.

Please, if you want to do CodeReview then better prepare a little bit more. You cannot pass class-instances for Reflection. Do imagine where this would end up?

I'll just say: $instance->get_class() Passing an instance is not only for convenience, you may want to prepare your service instance before exposing it and accepting requests...

you do not create instances when generating wsdl. Agreed, but I'm seeing a larger scope where your package could be the dispatcher of requests for various webservice protocols.

-Philippe