[PEPr] Comment on Payment::Payment_PayPalSoap

Joe Stump (http://pear.php.net/user/jstump) has commented on the proposal for Payment::Payment_PayPalSoap. Comment: 1.) I'm unsure if PEAR requires separate files for each exception (I saw a recent proposal that had all of them in one file), but I'd prefer to see a single Exception.php file with all exceptions in there or a lazy loader of exceptions rather than three separate include files (each requiring an fstat and fseek along with a loop through the include_path). 2.) By using nothing but private members/methods there's no way for me to do much of anything by way of extending or mocking up of the object. Travis Swicegood may want to chime in more here as he's the one who broke me of this habit, but I generally prefer protected over private these days. 3.) I have a slightly different pattern that I use for setters if you'd like to ping me offline I'll give you more details. I generally dislike the use of tons of setters now that PHP5 has __set() and I think there's a way to offer the flexibility you're looking for while not having a setter for each member variable (Daniel has my contact info). 4.) Why not use __call() for the example? e.g. $client->SetExpressCheckout(); 5.) In the call() method you throw $e, which is a SoapFault. I generally, when coding with PEAR, expect a package related exception to be thrown. Maybe add an unknown exception type and rethrow that with the message, etc.? Dunno. Just a thought. Not sure if the docs cover throwing non-package related exceptions. 6.) I think I'd change the package name. To what I don't know, but my top suggestions are: Payment_PayPal_SOAP and Services_PayPal_SOAP. I'd expect there to be other Payment_PayPal related packages in the future so it'd make sense that this be a "sub-package" of that maybe. 7.) There's no ways for me to modify any of the SOAP stuff before the SOAP client is loaded in __construct(). This is an argument for one of the following: 1.) Make the $_soapOptions array protected so I can override it in my child class (another argument for #2), 2.) Lazily load the SOAP client only when call() is ran along with a way to manipulate that data via a setOption() or some such method. I like the package. Concise and simple. Just the way I like my code :) Ping me offline about my setter pattern if you're interested. Thanks! Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=556 -- Sent by PEPr, the automatic proposal system at http://pear.php.net