clue / reactphp-soap

Simple, async SOAP webservice client, built on top of ReactPHP.
https://clue.engineering/2020/announcing-reactphp-soap-2
MIT License
64 stars 25 forks source link

Rename createClient() to something more meaningful #15

Closed clue closed 6 years ago

clue commented 9 years ago

The Factory::createClient() method could use a more meaningful method name.

This has originally been brought up in #11.

clue commented 9 years ago

Some points originally raised by @floriansimon1:

renaming the original function […] creates a compatibility break

Given this current number of dependents and the fact that this library is currently at v0.1.0, I'm okay with introducing a BC break and describing the upgrade path in the documentation.

The fact that createClient does not return a client is not a problem to me. It says that it's creating a client, not that it's returning it. I'm kind of biased though, I'm used to working with promises for everything :)

This is an interesting one :) I'm tempted to agree here, as this was also part of the initial motivation for the original naming. However, once the new createClientFromWsdl() lands (via #11), we should make this more obvious. The latter does in fact return a Client instance whereas the original method returns a Promise which resolves to a Client.

I can't say I particularly like createClientWsdl, because, it might let people think that it generates a WSDL file, which would be confusing IMO.

I'm having to agree here. This suggestion was a lame attempt to keep the method name within reasonable limits, but this does not make the method name any better :)

As such, my personal vote would be something (lengthy) like createDeferredClientFromWsdlUri().

Any input is welcome!

floriansimon1 commented 9 years ago

I second that !

clue commented 8 years ago

Thanks for the confirmation, feel like filing a PR? :-)

floriansimon1 commented 8 years ago

I will when I start working again on the project that uses this library!