You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2017/08/12 07:59:17 UTC
svn commit: r1804859 -
/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java
Author: jleroux
Date: Sat Aug 12 07:59:16 2017
New Revision: 1804859
URL: http://svn.apache.org/viewvc?rev=1804859&view=rev
Log:
Reverted: [FB] Package org.apache.ofbiz.accounting.payment (Additional Bugs)
(OFBIZ-9529)
GiftCertificateServices.java:229, DLS_DEAD_LOCAL_STORE
GiftCertificateServices.java:306, DLS_DEAD_LOCAL_STORE
DLS: Dead store to balance in
GiftCertificateServices.addFundsToGiftCertificate(DispatchContext, Map)
This instruction assigns a value to a local variable, but the value is not read
or used in any subsequent instruction. Often, this indicates an error,
because the value computed is never used.
It was late and I was too quick yesterday night. I see no reason why we would
replace ZERO by null (no assignment) in both cases, the variables are used.
So I revert r1804847 and I rather replace occurrences of BigDecimal.ZERO by the
locally defined constant ZERO
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=1804859&r1=1804858&r2=1804859&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 07:59:16 2017
@@ -226,7 +226,7 @@ public class GiftCertificateServices {
}
// create the transaction
- BigDecimal balance;
+ BigDecimal balance = ZERO;
String refNum = null;
try {
refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId, partyId,
@@ -270,7 +270,7 @@ public class GiftCertificateServices {
}
// validate the amount
- if (amount.compareTo(BigDecimal.ZERO) < 0) {
+ if (amount.compareTo(ZERO) < 0) {
return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
"AccountingFinAccountMustBePositive", locale));
}
@@ -301,9 +301,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 ? BigDecimal.ZERO : finAccount.getBigDecimal("actualBalance");
+ BigDecimal previousBalance = finAccount.get("actualBalance") == null ? ZERO : finAccount.getBigDecimal("actualBalance");
- BigDecimal balance;
+ BigDecimal balance = ZERO;
String refNum = null;
Boolean procResult;
if (previousBalance.compareTo(amount) >= 0) {
@@ -311,7 +311,7 @@ public class GiftCertificateServices {
refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId,
partyId, currencyUom, withdrawl, cardNumber, locale);
finAccount.refresh();
- balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
+ balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
procResult = Boolean.TRUE;
} catch (GeneralException e) {
Debug.logError(e, module);
@@ -356,7 +356,7 @@ public class GiftCertificateServices {
// TODO: get the real currency from context
// get the balance
- BigDecimal balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
+ BigDecimal balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
Map<String, Object> result = ServiceUtil.returnSuccess();
result.put("balance", balance);
Re: svn commit: r1804859 -
/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apac
he/ofbiz/accounting/payment/GiftCertificateServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks Gil,
Got it now :)
BTW what do you think about my 1st comment at https://issues.apache.org/jira/browse/OFBIZ-9572
Jacques
Le 14/08/2017 à 16:30, gil portenseigne a écrit :
> Hello Jacques,
>
> The reason why no assignment is good here, is that the assignment value will never be used, since the value is always overwritten with another
> assignment before it is used.
>
> It's the same with :
>
> String refNum = null;
>
> lines : 228 and 305.
>
> Gil
>
>
> On 12/08/2017 09:59, jleroux@apache.org wrote:
>> Author: jleroux
>> Date: Sat Aug 12 07:59:16 2017
>> New Revision: 1804859
>>
>> URL: http://svn.apache.org/viewvc?rev=1804859&view=rev
>> Log:
>> Reverted: [FB] Package org.apache.ofbiz.accounting.payment (Additional Bugs)
>> (OFBIZ-9529)
>>
>> GiftCertificateServices.java:229, DLS_DEAD_LOCAL_STORE
>> GiftCertificateServices.java:306, DLS_DEAD_LOCAL_STORE
>> DLS: Dead store to balance in
>> GiftCertificateServices.addFundsToGiftCertificate(DispatchContext, Map)
>> This instruction assigns a value to a local variable, but the value is not read
>> or used in any subsequent instruction. Often, this indicates an error,
>> because the value computed is never used.
>>
>> It was late and I was too quick yesterday night. I see no reason why we would
>> replace ZERO by null (no assignment) in both cases, the variables are used.
>> So I revert r1804847 and I rather replace occurrences of BigDecimal.ZERO by the
>> locally defined constant ZERO
>>
>> 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=1804859&r1=1804858&r2=1804859&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
>> 07:59:16 2017
>> @@ -226,7 +226,7 @@ public class GiftCertificateServices {
>> }
>> // create the transaction
>> - BigDecimal balance;
>> + BigDecimal balance = ZERO;
>> String refNum = null;
>> try {
>> refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId, partyId,
>> @@ -270,7 +270,7 @@ public class GiftCertificateServices {
>> }
>> // validate the amount
>> - if (amount.compareTo(BigDecimal.ZERO) < 0) {
>> + if (amount.compareTo(ZERO) < 0) {
>> return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>> "AccountingFinAccountMustBePositive", locale));
>> }
>> @@ -301,9 +301,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 ? BigDecimal.ZERO : finAccount.getBigDecimal("actualBalance");
>> + BigDecimal previousBalance = finAccount.get("actualBalance") == null ? ZERO : finAccount.getBigDecimal("actualBalance");
>> - BigDecimal balance;
>> + BigDecimal balance = ZERO;
>> String refNum = null;
>> Boolean procResult;
>> if (previousBalance.compareTo(amount) >= 0) {
>> @@ -311,7 +311,7 @@ public class GiftCertificateServices {
>> refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId,
>> partyId, currencyUom, withdrawl, cardNumber, locale);
>> finAccount.refresh();
>> - balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
>> + balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
>> procResult = Boolean.TRUE;
>> } catch (GeneralException e) {
>> Debug.logError(e, module);
>> @@ -356,7 +356,7 @@ public class GiftCertificateServices {
>> // TODO: get the real currency from context
>> // get the balance
>> - BigDecimal balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
>> + BigDecimal balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
>> Map<String, Object> result = ServiceUtil.returnSuccess();
>> result.put("balance", balance);
>>
>>
>
>
Re: svn commit: r1804859 -
/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/payment/GiftCertificateServices.java
Posted by gil portenseigne <gi...@nereide.fr>.
Hello Jacques,
The reason why no assignment is good here, is that the assignment value
will never be used, since the value is always overwritten with another
assignment before it is used.
It's the same with :
String refNum = null;
lines : 228 and 305.
Gil
On 12/08/2017 09:59, jleroux@apache.org wrote:
> Author: jleroux
> Date: Sat Aug 12 07:59:16 2017
> New Revision: 1804859
>
> URL: http://svn.apache.org/viewvc?rev=1804859&view=rev
> Log:
> Reverted: [FB] Package org.apache.ofbiz.accounting.payment (Additional Bugs)
> (OFBIZ-9529)
>
> GiftCertificateServices.java:229, DLS_DEAD_LOCAL_STORE
> GiftCertificateServices.java:306, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to balance in
> GiftCertificateServices.addFundsToGiftCertificate(DispatchContext, Map)
> This instruction assigns a value to a local variable, but the value is not read
> or used in any subsequent instruction. Often, this indicates an error,
> because the value computed is never used.
>
> It was late and I was too quick yesterday night. I see no reason why we would
> replace ZERO by null (no assignment) in both cases, the variables are used.
> So I revert r1804847 and I rather replace occurrences of BigDecimal.ZERO by the
> locally defined constant ZERO
>
> 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=1804859&r1=1804858&r2=1804859&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 07:59:16 2017
> @@ -226,7 +226,7 @@ public class GiftCertificateServices {
> }
>
> // create the transaction
> - BigDecimal balance;
> + BigDecimal balance = ZERO;
> String refNum = null;
> try {
> refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId, partyId,
> @@ -270,7 +270,7 @@ public class GiftCertificateServices {
> }
>
> // validate the amount
> - if (amount.compareTo(BigDecimal.ZERO) < 0) {
> + if (amount.compareTo(ZERO) < 0) {
> return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
> "AccountingFinAccountMustBePositive", locale));
> }
> @@ -301,9 +301,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 ? BigDecimal.ZERO : finAccount.getBigDecimal("actualBalance");
> + BigDecimal previousBalance = finAccount.get("actualBalance") == null ? ZERO : finAccount.getBigDecimal("actualBalance");
>
> - BigDecimal balance;
> + BigDecimal balance = ZERO;
> String refNum = null;
> Boolean procResult;
> if (previousBalance.compareTo(amount) >= 0) {
> @@ -311,7 +311,7 @@ public class GiftCertificateServices {
> refNum = GiftCertificateServices.createTransaction(delegator, dispatcher, userLogin, amount, productStoreId,
> partyId, currencyUom, withdrawl, cardNumber, locale);
> finAccount.refresh();
> - balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
> + balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
> procResult = Boolean.TRUE;
> } catch (GeneralException e) {
> Debug.logError(e, module);
> @@ -356,7 +356,7 @@ public class GiftCertificateServices {
>
> // TODO: get the real currency from context
> // get the balance
> - BigDecimal balance = finAccount.get("availableBalance") == null ? BigDecimal.ZERO : finAccount.getBigDecimal("availableBalance");
> + BigDecimal balance = finAccount.get("availableBalance") == null ? ZERO : finAccount.getBigDecimal("availableBalance");
>
> Map<String, Object> result = ServiceUtil.returnSuccess();
> result.put("balance", balance);
>
>