-1
\$\begingroup\$

Please help me restructure this Java code to allow for sometime have 4 items in packagePriceList.

public static HTMLSelectOptionsElement listPremiumPackageTermsWithPrice(String[] packagePriceList, int cyberSourceBillingMode) {
    HTMLSelectOptionsElement optionsElement = new HTMLSelectOptionsElement();

    // Testing only. Allows for a shorter recurring term for QA testing.
    if (cyberSourceBillingMode == DiceUtilConstants.CYBERSOURCE_BILLING_MODE_ICS_WEEKLY_OVERRIDE) {
        optionsElement.addOption("1 Week", String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_7_DAYS));
    }
    optionsElement.addOption("1 Month - " + packagePriceList[0], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_30_DAYS));
    optionsElement.addOption("3 Months - " + packagePriceList[1], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_90_DAYS));
    optionsElement.addOption("6 Months - " + packagePriceList[2], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_180_DAYS));
    optionsElement.addOption("12 Months - " + packagePriceList[3], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_365_DAYS));

    return optionsElement;
}

I thought about just catching the IndexOutOfBoundsException but would that be a bit of a hack?

\$\endgroup\$
3
  • \$\begingroup\$ The question is not clear: you'll always get packagePriceList of length 4, or sometimes less, sometimes more? \$\endgroup\$
    – toto2
    May 14, 2013 at 19:16
  • \$\begingroup\$ Also thought about adding a completely new method or adding a check in the method on the size of packagePriceList. \$\endgroup\$ May 14, 2013 at 19:26
  • \$\begingroup\$ Yes toto2 the packagePriceList sometimes has 3 and sometimes 4. I apologize for the question not being clear. \$\endgroup\$ May 14, 2013 at 19:28

1 Answer 1

1
\$\begingroup\$

First, a simple solution:

optionsElement.addOption("1 Month - " + packagePriceList[0], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_30_DAYS));
optionsElement.addOption("3 Months - " + packagePriceList[1], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_90_DAYS));
optionsElement.addOption("6 Months - " + packagePriceList[2], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_180_DAYS));
if (packagePriceList.length == 4)
   optionsElement.addOption("12 Months - " + packagePriceList[3], String.valueOf(UserPackage.PREMIUM_PACKAGE_TERM_365_DAYS));

But I don't like the signature of listPremiumPackageTermsWithPrice(String[] packagePriceList,...). The problem is that you must implicitly know that the the first element corresponds to 1 month, etc. It would be better to have an argument which is of type Map<PackageTerm, String>, where the key of the map is one of UserPackage.PREMIUM_PACKAGE_TERM_XX_DAYS and the value of the map is the same string as before. If your UserPackage.PREMIUM_PACKAGE_TERM_XX_DAYS where actually enum's, you could use an EnumMap.

Here is the enum I would define:

public enum PackageTerm {
  PREMIUM_PACKAGE_TERM_30_DAYS("1 month"), 
  PREMIUM_PACKAGE_TERM_90_DAYS("3 months"), 
  PREMIUM_PACKAGE_TERM_180_DAYS("6 months"), 
  PREMIUM_PACKAGE_TERM_365_DAYS("12 months");

  private String timePeriodString;

  private PackageTerm(String timePeriodString) {
     this.timePeriodString = timePeriodString);
  }

  public String printedString(String price) {
    return timePeriodString + " - " + price;
  }
}

and the method is changed to:

HTMLSelectOptionsElement listPremiumPackageTermsWithPrice(Map<PackageTerm, String> packagesWithPrice, ...) {
   for (PackageTerm packageTerm : PackageTerm.values()) { // in the order 1/3/6/12 months
      String priceIfAny = packagesWithPrice.get(packageTerm); // null if not present
      if (priceIfAny != null)
         optionsElement.addOptions(packageTerm.printedString(priceIfAny), packageTerm.toString());
         // You would need to override PackageTerm.toString().  By default it is the name of the enum.
   }
}

Note that you could put other relevant data and methods within the enum PackageTerm.

\$\endgroup\$
1
  • \$\begingroup\$ Not coherent number of days PREMIUM_PACKAGE_TERM_30_DAYS("6 months"), PREMIUM_PACKAGE_TERM_30_DAYS("12 months"); \$\endgroup\$
    – cl-r
    May 15, 2013 at 7:53

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