Accepting any object and returning the object

Question

I have been tinkering with generic methods in Java but still haven't figured out a way to optimize this code. Here I need to write similar piece of code for both plan and add-on.

public Set<SubscriptionDto> getCustomerSubscriptions(Integer customerId) {
 Set<SubscriptionDto> subscribedProducts = new HashSet<>();
 Set<PlanDto> subscribedPlans = new HashSet<>();
 Set<AddonDto> subscribedAddons = new HashSet<>();
 Double totalPrice = (double) 0;
 try {
 List<Subscription> subscriptions = subscriptionService.findByCustomerId(customerId);
 if (subscriptions != null) {
 for (Subscription subscription : subscriptions) {
 SubscriptionDto subscriptionDto = new SubscriptionDto();
 // Get the Plans subscribed for particular product
 for (SubscribedLicense subscribedLicense : subscription.getSubscribedLicenses()) {
 // Get Plan
 if (subscribedLicense.getLicenseType().equals(LicenseTypeEnum.PLAN.getDescription())) {
 Plan plan = planService.findById(subscribedLicense.getLicenseTypeId());
 PlanDto planDto = new PlanDto();
 planDto.setPlanName(plan.getName());
 planDto.setNoOfFreeLicense(subscribedLicense.getNoOfFreeLicense());
 planDto.setNoOfLicense(subscribedLicense.getNoOfLicense());
 subscribedPlans.add(planDto);
 // Calculate Price
 PlanPrice planPrice = planPriceService.findById(subscribedLicense.getLicenseTypePriceId());
 if (planPrice != null) {
 totalPrice = totalPrice + (planDto.getNoOfLicense() * planPrice.getPrice());
 }
 }
 // Get Addon
 if (subscribedLicense.getLicenseType().equals(LicenseTypeEnum.ADDON.getDescription())) {
 Addon addon = addonService.findById(subscribedLicense.getLicenseTypeId());
 AddonDto addonDto = new AddonDto();
 addonDto.setAddonName(addon.getName());
 addonDto.setNoOfFreeLicense(subscribedLicense.getNoOfFreeLicense());
 addonDto.setNoOfLicense(subscribedLicense.getNoOfLicense());
 subscribedAddons.add(addonDto);
 // Calculate Price
 AddonPrice addonPrice = addonPriceService
 .findById(subscribedLicense.getLicenseTypePriceId());
 if (addonPrice != null) {
 totalPrice = totalPrice + (addonDto.getNoOfLicense() * addonPrice.getPrice());
 }
 }
 }
 // Add price for the subscription
 subscriptionDto.setTotalPrice(totalPrice);
 // Add Product Name
 subscriptionDto.setProductName(subscription.getProduct().getName());
 // Add Product Image
 subscriptionDto.setProductImage(subscription.getProduct().getImageName());
 // Add Billing Period Type
 subscriptionDto.setBillingPeriodType(subscription.getBillingPeriodType());
 // Add plans customer has subscribed
 if (!subscribedPlans.isEmpty()) {
 subscriptionDto.setPlanDtos(subscribedPlans);
 }
 // Add addons customer has subscribed
 if (!subscribedAddons.isEmpty()) {
 subscriptionDto.setAddonDtos(subscribedAddons);
 }
 // Add subscriptions to subscriptions list
 subscribedProducts.add(subscriptionDto);
 }
 }
 return subscribedProducts;
 } catch (Exception ex) {
 }
}

• BCdotWEB: As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles.
• mdfst13: Hi. Welcome to Code Review! Titles here should tell what the code does, not what you want from the review. You also may want to explain at greater length in the body of the post what the code is intended to do. Also, why do you need to optimize it? What behavior are you observing that suggests that this method needs optimized? There isn't much context here. What if the performance problem is in subscriptionService? You probably won't fix that in this code.

Answer 1

That nested for loop looks prime for some Java 8 stream refactor.

Also, I would drop the isEmpty checks and just make sure the client can understand an empty collection being present.

That massive try block looks ugly and is too generic anyway. Change the method signature (if possible) to throw more relevant Exceptions.

totalPrice looks like it would fit into some sort of Java 8 aggregation function.