braintree / braintree_java

Braintree Java library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
158 stars 98 forks source link

Moved method getSubscriptions to PaymentMethod interface #38

Closed singhalkul closed 8 years ago

singhalkul commented 8 years ago

By having getSubscriptions method in the interfacet, it is possible to get all subscriptions of a user easily instead of having to cast for each and every payment method implementation.

omgrr commented 8 years ago

Is there a particular use case for this? Unless there is a lot of value, we typically shy away from adding short hand methods. Can you give us an example of where you are using this and what it looks like without it?

matthewarkin commented 8 years ago

I found this a bit confusing but then I realized my own frustration with it a while back.

Psuedo code - New Way:

List<Subscriptions> subs =  new List<Subscriptions>();
List<PaymentMethods>  paymentmethods = Customer.paymentMethods;
foreach (PaymentMethod p in paymentMethods){
    subs.add(p.getSubscriptions);
}

Psuedo code - oldway Way:

List<Subscriptions> subs =  new List<Subscriptions>();
List<CreditCards>  cards = Customer.creditcards;
foreach (CreditCard p in cards){
    subs.add(p.getSubscriptions);
}
List<PaypalAccount>  pp = Customer.paypalaccounts;
foreach (PaypalAccount p in pp){
    subs.add(p.getSubscriptions);
}
...
siddharth178 commented 8 years ago

Exactly, the new way certainly helps in fetching customer subscriptions from all payment methods even when some payment methods get added or removed.

singhalkul commented 8 years ago

@omgrr The use case is that we want to show all the subscriptions of a customer across all the payment methods. This functionality is available on the braintree control panel as well.

As @matthewarkin mentioned in his comment, if the getSubscriptions method is not present in the PaymentMethod interface, then we will have to get them from each and every payment method which means a lot of code and we will have to modify our code everytime a new payment method is added or some existing one is removed.

omgrr commented 8 years ago

Sorry for the slow response. We talked it over internally and I think we're with you that this makes sense. We just need to add some tests around it and will let you know if we have any questions or news.

danakatz commented 8 years ago

@singhalkul This has been included in the 2.60.0 release. Thank you for contributing!