Re: svn commit: r1804864 - /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apac he/ofbiz/accounting/payment/GiftCertificateServices.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1804864 - /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apac he/ofbiz/accounting/payment/GiftCertificateServices.java

Jacques Le Roux
Administrator
Hi Michael, All,

Actually I think a better use of ZERO is possible. See my 1st comment at https://issues.apache.org/jira/browse/OFBIZ-9572

I believe the case we have in GiftCertificateServices class (no use of setScale() like below) and in few other classes is simply cargo cult w/o use of
setScale().

public static final BigDecimal ZERO = BigDecimal.ZERO.setScale(decimals, rounding);

Setting setScale in all cases should be the way, for instance when you compare as below

Jacques


Le 12/08/2017 à 13:52, [hidden email] a écrit :

> Author: mbrohl
> Date: Sat Aug 12 11:52:45 2017
> New Revision: 1804864
>
> URL: http://svn.apache.org/viewvc?rev=1804864&view=rev
> Log:
> Improved: Replaced unnecessary local variable ZERO with BigDecimal.ZERO.
> (OFBIZ-9529)
>
> Modified:
>      ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java
>
> Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java?rev=1804864&r1=1804863&r2=1804864&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java (original)
> +++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java Sat Aug 12 11:52:45 2017
> @@ -56,8 +56,6 @@ public class GiftCertificateServices {
>       public static final int CARD_NUMBER_LENGTH = 14;
>       public static final int PIN_NUMBER_LENGTH = 6;
>  
> -    public static final BigDecimal ZERO = BigDecimal.ZERO;
> -
>       // Base Gift Certificate Services
>       public static Map<String, Object> createGiftCertificate(DispatchContext dctx, Map<String, ? extends Object> context) {
>           LocalDispatcher dispatcher = dctx.getDispatcher();
> @@ -220,13 +218,13 @@ public class GiftCertificateServices {
>           }
>  
>           // get the previous balance
> -        BigDecimal previousBalance = ZERO;
> +        BigDecimal previousBalance = BigDecimal.ZERO;
>           if (finAccount.get("availableBalance") != null) {
>               previousBalance = finAccount.getBigDecimal("availableBalance");
>           }
>  
>           // create the transaction
> -        BigDecimal balance = ZERO;
> +        BigDecimal balance = BigDecimal.ZERO;
>           String refNum = null;
>           try {
>               refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId, partyId,
> @@ -270,7 +268,7 @@ public class GiftCertificateServices {
>           }
>  
>           // validate the amount
> -        if (amount.compareTo(ZERO) < 0) {
> +        if (amount.compareTo(BigDecimal.ZERO) < 0) {
>               return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>                       "AccountingFinAccountMustBePositive", locale));
>           }
> @@ -301,9 +299,9 @@ public class GiftCertificateServices {
>           }
>  
>           // check the actual balance (excluding authorized amounts) and create the transaction if it is sufficient
> -        BigDecimal previousBalance = finAccount.get("actualBalance") == null ? ZERO : finAccount.getBigDecimal("actualBalance");
> +        BigDecimal previousBalance = finAccount.get("actualBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("actualBalance");
>  
> -        BigDecimal balance = ZERO;
> +        BigDecimal balance = BigDecimal.ZERO;
>           String refNum = null;
>           Boolean procResult;
>           if (previousBalance.compareTo(amount) >= 0) {
> @@ -311,7 +309,7 @@ public class GiftCertificateServices {
>                   refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId,
>                           partyId, currencyUom, withdrawl, cardNumber, locale);
>                   finAccount.refresh();
> -                balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
> +                balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
>                   procResult = Boolean.TRUE;
>               } catch (GeneralException e) {
>                   Debug.logError(e, module);
> @@ -356,7 +354,7 @@ public class GiftCertificateServices {
>  
>           // TODO: get the real currency from context
>           // get the balance
> -        BigDecimal balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
> +        BigDecimal balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
>  
>           Map<String, Object> result = ServiceUtil.returnSuccess();
>           result.put("balance", balance);
>
>
>