DivideBV / Postnl

Library to connect to PostNL's SOAP service called CIF
GNU General Public License v2.0
31 stars 40 forks source link

Ability to call client functions not wrapped in main class #36

Closed tomhooijenga closed 8 years ago

tomhooijenga commented 8 years ago

In the Postnl class, there are setters to set the various clients. However, there are no getters for them. Because of this, there is no easy way to call some parts of the api. (For example, completeStatus from ShippingStatusClient). Two possible fixes: Proxy all methods from each client in the Postnl class, or make the clients themselves accessible.

ameenross commented 8 years ago

Actually, there's getClient. It's just protected is all. I have considered making it public in the past, but decided not to, because I figured the main class might only be an obstacle for a developer with more custom needs.

I don't think proxying all methods is a good idea, unless the wrapper function actually adds value (which is the case for the functions currently in there). Making the getClient function public is probably the best solution.

tomhooijenga commented 8 years ago

I think it it is more convenient to have the getClient method public. That way, there is a single access point to the api. This also prevents the consumer from dealing with the SecurityHeader. Are there any downsides in doing this?

ameenross commented 8 years ago

Are there any downsides in doing this?

Probably only psychological. In other words, no :)

ameenross commented 8 years ago

On second thought, that would result in code like this (and wouldn't catch CIF exceptions):

$request = new ComplexTypes\CompleteStatusRequest($message, $customer, $shipment);

// Query the webservice and return the result.
return $postnl->getClient('ShippingStatusClient')->completeStatus($request);

Making the call method available would be preferable, and using that will result in something like this (AND have proper CIF error handling):

$request = new ComplexTypes\CompleteStatusRequest($message, $customer, $shipment);

// Query the webservice and return the result.
return $postnl->call('ShippingStatusClient', 'completeStatus', $request);

If you agree then I'll patch it.

tomhooijenga commented 8 years ago

That is probably better. Great work!