On Tue, Apr 28, 2009 at 6:48 PM, Vadim Gabriel <vadim...@gmail.com> wrote: > Hey, > > Wow. Talking about feedback. Those are some great pointers you have right > there. I will tackle them starting tomorrow to make it as much as developer > friendly as possible. Now to your pointers > > About the first question you had with the notification upon payment or > subscription. From what i have read about it on paypal's developer central > and across the web is that you specify a return url or a notify_url that > paypal 'pings' or silently callbacks and that link can just be an action in > a controller that uses one of the methods to process the payment. I am not > sure if this is the exact case but in my testing (using sandbox) my > subscriptions were processed fine. So more testing on this or ideas will be > required i assume. To receive notifications based on the way your processing > the request the _parseIPNResponse will be used for 3 methods that actually > need it and they are documented in the class. I am not that familiar with > Paypal so if anyone willing to give this a go and test/edit the code he is > more then welcome. > > (...) > 5) What if i want to get the returned array and use it's contents for > storing? I see the point of having isVerfiedAddress but wouldn't it be a > better idea to have both then just returning a single boolean value?
That would be a different method though. I mean, personally, I have nothing against anyone storing the data, I just meant that is*() methods should be reserved for true/false cases. I'm following PHP basically -- is_file, is_readable, is_array, ... > 6) I was thinking about that while i was wrtting this. But then i said for > consistency i should stick with one class for each gateway. It's not a > problem to split those up to there appropriate classes. That's why i wanted > this feedback. The question is if this is something that more developers > would like to see? And in what way to split them up? IMHO, it would be nice since it makes the API more simple and cleaner. > 7) They maybe not named the way you said it but by default they return the > link not redirect. If you pass a boolean value of true to the $autoRedirect > then it will redirect. Both options are available. That's a fair compromise. ;-) > (...) > 9) Ahh i tried really hard on this one (phpdoc) and i provided the right > phpdoc style for the file, class, methods and property names. What did i > miss? Could you be more specific? Oh, basically I think you could improve the documentation by completing it. I also like @see, or @uses. I'm not so sure what the ZF CS requires or allows. Best double-check the manual. Some of the descriptions could be more detailed (I thought), but sometimes an easier name for a method or maybe breaking it up into sub classes would make that irrelevant. > 10) Yes, I was thinking if throwing exceptions will be a better choice then > just storing the error message returned in a class variables and then > retriving them later on. At some point i even included both. Do you think > throwing exceptions will be best handled? If so i could probably throw them > then just storing the message in a varible. Maybe allow people to control the behavior? > 11) Actually that's already exists. and used in the Tranzila gateway. If you > take a look at the Zend_Payment_Gateway_Abstract you will notice them You're right, I oversaw that. Then my request would be to use them everywhere in order to make it consistent. > (...) > Thanks again for the feedback. And if you or anyone else would like to help > out, It will be apprecaited. I can give feedback always. That's for sure! Also, we are in the process of evaluating different providers right now. Maybe I can contribute another gateway. Is Tranzila any good? Cheers, Till
