Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

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) {

    }
}
share|improve this question
    
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. – BCdotWEB May 20 at 7:37
1  
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. – mdfst13 May 20 at 7:38

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.