You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by su...@apache.org on 2020/06/13 08:56:15 UTC

[ofbiz-framework] branch trunk updated: Improved: Changed decimals, rounding, zero static variables names as per best practices. (#195)

This is an automated email from the ASF dual-hosted git repository.

surajk pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
     new a527862  Improved: Changed decimals, rounding, zero static variables names as per best practices. (#195)
a527862 is described below

commit a52786296100efe67b77eb2c2e44efcb27a495b6
Author: Suraj Khurana <64...@users.noreply.github.com>
AuthorDate: Sat Jun 13 14:26:06 2020 +0530

    Improved: Changed decimals, rounding, zero static variables names as per best practices. (#195)
    
    (OFBIZ-11804)
    Also make them private data members of the class.
---
 .../apache/ofbiz/product/price/PriceServices.java  | 22 ++++-----
 .../ofbiz/shipment/shipment/ShipmentServices.java  | 14 +++---
 .../ofbiz/shipment/thirdparty/ups/UpsServices.java | 57 +++++++++++-----------
 .../weightPackage/WeightPackageSession.java        |  4 +-
 4 files changed, 48 insertions(+), 49 deletions(-)

diff --git a/applications/product/src/main/java/org/apache/ofbiz/product/price/PriceServices.java b/applications/product/src/main/java/org/apache/ofbiz/product/price/PriceServices.java
index 390bd51..a424b5a 100644
--- a/applications/product/src/main/java/org/apache/ofbiz/product/price/PriceServices.java
+++ b/applications/product/src/main/java/org/apache/ofbiz/product/price/PriceServices.java
@@ -57,12 +57,12 @@ public class PriceServices {
 
     private static final String MODULE = PriceServices.class.getName();
     private static final String RESOURCE = "ProductUiLabels";
-    public static final BigDecimal ONE_BASE = BigDecimal.ONE;
-    public static final BigDecimal PERCENT_SCALE = new BigDecimal("100.000");
+    private static final BigDecimal ONE_BASE = BigDecimal.ONE;
+    private static final BigDecimal PERCENT_SCALE = new BigDecimal("100.000");
 
-    public static final int taxCalcScale = UtilNumber.getBigDecimalScale("salestax.calc.decimals");
-    public static final int taxFinalScale = UtilNumber.getBigDecimalScale("salestax.final.decimals");
-    public static final RoundingMode taxRounding = UtilNumber.getRoundingMode("salestax.rounding");
+    private static final int TAX_SCALE = UtilNumber.getBigDecimalScale("salestax.calc.decimals");
+    private static final int TAX_FINAL_SCALE = UtilNumber.getBigDecimalScale("salestax.final.decimals");
+    private static final RoundingMode TAX_ROUNDING = UtilNumber.getRoundingMode("salestax.rounding");
 
     /**
      * <p>Calculates the price of a product from pricing rules given the following input, and of course access to the database:</p>
@@ -599,21 +599,21 @@ public class PriceServices {
 
                 // based on the taxPercentage calculate the other amounts, including: listPrice, defaultPrice, averageCost, promoPrice, competitivePrice
                 BigDecimal taxPercentage = (BigDecimal) calcTaxForDisplayResult.get("taxPercentage");
-                BigDecimal taxMultiplier = ONE_BASE.add(taxPercentage.divide(PERCENT_SCALE, taxCalcScale, taxRounding));
+                BigDecimal taxMultiplier = ONE_BASE.add(taxPercentage.divide(PERCENT_SCALE, TAX_SCALE, TAX_ROUNDING));
                 if (result.get("listPrice") != null) {
-                    result.put("listPrice", ((BigDecimal) result.get("listPrice")).multiply(taxMultiplier).setScale(taxFinalScale, taxRounding));
+                    result.put("listPrice", ((BigDecimal) result.get("listPrice")).multiply(taxMultiplier).setScale(TAX_FINAL_SCALE, TAX_ROUNDING));
                 }
                 if (result.get("defaultPrice") != null) {
-                    result.put("defaultPrice", ((BigDecimal) result.get("defaultPrice")).multiply(taxMultiplier).setScale(taxFinalScale, taxRounding));
+                    result.put("defaultPrice", ((BigDecimal) result.get("defaultPrice")).multiply(taxMultiplier).setScale(TAX_FINAL_SCALE, TAX_ROUNDING));
                 }
                 if (result.get("averageCost") != null) {
-                    result.put("averageCost", ((BigDecimal) result.get("averageCost")).multiply(taxMultiplier).setScale(taxFinalScale, taxRounding));
+                    result.put("averageCost", ((BigDecimal) result.get("averageCost")).multiply(taxMultiplier).setScale(TAX_FINAL_SCALE, TAX_ROUNDING));
                 }
                 if (result.get("promoPrice") != null) {
-                    result.put("promoPrice", ((BigDecimal) result.get("promoPrice")).multiply(taxMultiplier).setScale(taxFinalScale, taxRounding));
+                    result.put("promoPrice", ((BigDecimal) result.get("promoPrice")).multiply(taxMultiplier).setScale(TAX_FINAL_SCALE, TAX_ROUNDING));
                 }
                 if (result.get("competitivePrice") != null) {
-                    result.put("competitivePrice", ((BigDecimal) result.get("competitivePrice")).multiply(taxMultiplier).setScale(taxFinalScale, taxRounding));
+                    result.put("competitivePrice", ((BigDecimal) result.get("competitivePrice")).multiply(taxMultiplier).setScale(TAX_FINAL_SCALE, TAX_ROUNDING));
                 }
             } catch (GenericServiceException e) {
                 Debug.logError(e, "Error calculating VAT tax (with calcTaxForDisplay service): " + e.toString(), MODULE);
diff --git a/applications/product/src/main/java/org/apache/ofbiz/shipment/shipment/ShipmentServices.java b/applications/product/src/main/java/org/apache/ofbiz/shipment/shipment/ShipmentServices.java
index 27182b1..423a1cc 100644
--- a/applications/product/src/main/java/org/apache/ofbiz/shipment/shipment/ShipmentServices.java
+++ b/applications/product/src/main/java/org/apache/ofbiz/shipment/shipment/ShipmentServices.java
@@ -57,12 +57,12 @@ import org.apache.ofbiz.service.ServiceUtil;
 public class ShipmentServices {
 
     private static final String MODULE = ShipmentServices.class.getName();
-
     private static final String RESOURCE = "ProductUiLabels";
     private static final String RES_ERROR = "OrderErrorUiLabels";
-    public static final int decimals = UtilNumber.getBigDecimalScale("order.decimals");
-    public static final RoundingMode rounding = UtilNumber.getRoundingMode("order.rounding");
-    public static final BigDecimal ZERO = BigDecimal.ZERO.setScale(decimals, rounding);
+
+    private static final int DECIMALS = UtilNumber.getBigDecimalScale("order.decimals");
+    private static final RoundingMode ROUNDING = UtilNumber.getRoundingMode("order.rounding");
+    private static final BigDecimal ZERO = BigDecimal.ZERO.setScale(DECIMALS, ROUNDING);
 
     public static Map<String, Object> createShipmentEstimate(DispatchContext dctx, Map<String, ? extends Object> context) {
         Map<String, Object> result = new HashMap<>();
@@ -1011,10 +1011,10 @@ public class ShipmentServices {
 
                 // How much of the invoiced quantity does the issued quantity represent?
                 BigDecimal issuedQuantity = packageContent.getBigDecimal("issuedQuantity");
-                BigDecimal proportionOfInvoicedQuantity = invoicedQuantity.signum() == 0 ? ZERO : issuedQuantity.divide(invoicedQuantity, 10, rounding);
+                BigDecimal proportionOfInvoicedQuantity = invoicedQuantity.signum() == 0 ? ZERO : issuedQuantity.divide(invoicedQuantity, 10, ROUNDING);
 
                 // Prorate the orderItem's invoiced amount by that proportion
-                BigDecimal packageContentValue = proportionOfInvoicedQuantity.multiply(invoicedAmount).setScale(decimals, rounding);
+                BigDecimal packageContentValue = proportionOfInvoicedQuantity.multiply(invoicedAmount).setScale(DECIMALS, ROUNDING);
 
                 // Convert the value to the shipment currency, if necessary
                 GenericValue orderHeader = packageContent.getRelatedOne("OrderHeader", false);
@@ -1023,7 +1023,7 @@ public class ShipmentServices {
                     return ServiceUtil.returnError(ServiceUtil.getErrorMessage(convertUomResult));
                 }
                 if (convertUomResult.containsKey("convertedValue")) {
-                    packageContentValue = ((BigDecimal) convertUomResult.get("convertedValue")).setScale(decimals, rounding);
+                    packageContentValue = ((BigDecimal) convertUomResult.get("convertedValue")).setScale(DECIMALS, ROUNDING);
                 }
 
                 // Add the value of the packed item to the package's total value
diff --git a/applications/product/src/main/java/org/apache/ofbiz/shipment/thirdparty/ups/UpsServices.java b/applications/product/src/main/java/org/apache/ofbiz/shipment/thirdparty/ups/UpsServices.java
index a40b1e2..ded7bdc 100644
--- a/applications/product/src/main/java/org/apache/ofbiz/shipment/thirdparty/ups/UpsServices.java
+++ b/applications/product/src/main/java/org/apache/ofbiz/shipment/thirdparty/ups/UpsServices.java
@@ -78,25 +78,24 @@ import org.xml.sax.SAXException;
  */
 public class UpsServices {
 
-    public final static String MODULE = UpsServices.class.getName();
+    private static final String MODULE = UpsServices.class.getName();
+    private static final String RES_ERROR = "ProductUiLabels";
+    private static final String RES_ORDER = "OrderUiLabels";
 
-    private static final Map<String, String> unitsUpsToOfbiz = new HashMap<>();
-    private static final Map<String, String> unitsOfbizToUps = new HashMap<>();
-    static {
-        unitsUpsToOfbiz.put("LBS", "WT_lb");
-        unitsUpsToOfbiz.put("KGS", "WT_kg");
+    private static final Map<String, String> UPS_TO_OFBIZ = new HashMap<>();
+    private static final Map<String, String> OFBIZ_TO_UPS = new HashMap<>();
+    private static final int DECIMALS = UtilNumber.getBigDecimalScale("order.decimals");
+    private static final RoundingMode ROUNDING = UtilNumber.getRoundingMode("order.rounding");
+    private static final int RET_SERVICE_CODE = 8;
+    private static final String DATE_FORMAT = "yyyyMMdd";
 
-        for (Map.Entry<String, String> entry: unitsUpsToOfbiz.entrySet()) {
-            unitsOfbizToUps.put(entry.getValue(), entry.getKey());
+    static {
+        UPS_TO_OFBIZ.put("LBS", "WT_lb");
+        UPS_TO_OFBIZ.put("KGS", "WT_kg");
+        for (Map.Entry<String, String> entry: UPS_TO_OFBIZ.entrySet()) {
+            OFBIZ_TO_UPS.put(entry.getValue(), entry.getKey());
         }
     }
-    public static final int decimals = UtilNumber.getBigDecimalScale("order.decimals");
-    public static final RoundingMode rounding = UtilNumber.getRoundingMode("order.rounding");
-    public static final MathContext generalRounding = new MathContext(10);
-    public static final int returnServiceCode = 8;
-    public static final String dateFormatString = "yyyyMMdd";
-    private static final String RES_ERROR = "ProductUiLabels";
-    private static final String RES_ORDER = "OrderUiLabels";
 
     public static Map<String, Object> upsShipmentConfirm(DispatchContext dctx, Map<String, ? extends Object> context) {
         Delegator delegator = dctx.getDelegator();
@@ -287,9 +286,9 @@ public class UpsServices {
                 if (codSurchargeApplyToNoPackages) {
                     codSurchargeAmount = "0";
                 }
-                codSurchargePackageAmount = new BigDecimal(codSurchargeAmount).setScale(decimals, rounding);
+                codSurchargePackageAmount = new BigDecimal(codSurchargeAmount).setScale(DECIMALS, ROUNDING);
                 if (codSurchargeSplitBetweenPackages) {
-                    codSurchargePackageAmount = codSurchargePackageAmount.divide(new BigDecimal(shipmentPackageRouteSegs.size()), decimals, rounding);
+                    codSurchargePackageAmount = codSurchargePackageAmount.divide(new BigDecimal(shipmentPackageRouteSegs.size()), DECIMALS, ROUNDING);
                 }
 
                 if (UtilValidate.isEmpty(destTelecomNumber)) {
@@ -466,7 +465,7 @@ public class UpsServices {
                     Element productElement = UtilXml.addChildElement(internationalFormsElement, "Product", shipmentConfirmRequestDoc);
                     UtilXml.addChildElementValue(productElement, "Description", "Product Description", shipmentConfirmRequestDoc);
                     Element unitElement = UtilXml.addChildElement(productElement, "Unit", shipmentConfirmRequestDoc);
-                    BigDecimal productQuantity = shipmentItem.getBigDecimal("quantity").setScale(decimals, rounding);
+                    BigDecimal productQuantity = shipmentItem.getBigDecimal("quantity").setScale(DECIMALS, ROUNDING);
                     UtilXml.addChildElementValue(unitElement, "Number", String.valueOf(productQuantity.intValue()), shipmentConfirmRequestDoc);
                     List<GenericValue> shipmentItemIssuances = shipmentItem.getRelated("ItemIssuance", null, null, false);
                     GenericValue orderItem = EntityUtil.getFirst(shipmentItemIssuances).getRelatedOne("OrderItem", false);
@@ -475,7 +474,7 @@ public class UpsServices {
                     UtilXml.addChildElementValue(unitOfMeasurElement, "Code", "EA", shipmentConfirmRequestDoc);
                     UtilXml.addChildElementValue(productElement, "OriginCountryCode", "US", shipmentConfirmRequestDoc);
                 }
-                SimpleDateFormat formatter = new SimpleDateFormat(dateFormatString);
+                SimpleDateFormat formatter = new SimpleDateFormat(DATE_FORMAT);
                 String invoiceDate = formatter.format(shipment.getTimestamp("createdDate"));
                 UtilXml.addChildElementValue(internationalFormsElement, "InvoiceDate", invoiceDate, shipmentConfirmRequestDoc);
                 UtilXml.addChildElementValue(internationalFormsElement, "ReasonForExport","SALE", shipmentConfirmRequestDoc);
@@ -538,7 +537,7 @@ public class UpsServices {
                 Element packageWeightUnitOfMeasurementElement = UtilXml.addChildElement(packageElement, "UnitOfMeasurement", shipmentConfirmRequestDoc);
                 String weightUomUps = null;
                 if (shipmentPackage.get("weightUomId") != null) {
-                    weightUomUps = unitsOfbizToUps.get(shipmentPackage.get("weightUomId"));
+                    weightUomUps = OFBIZ_TO_UPS.get(shipmentPackage.get("weightUomId"));
                 }
                 if (weightUomUps != null) {
                     UtilXml.addChildElementValue(packageWeightUnitOfMeasurementElement, "Code", weightUomUps, shipmentConfirmRequestDoc);
@@ -590,7 +589,7 @@ public class UpsServices {
                     Map<String, Object> convertUomResult = dispatcher.runSync("convertUom", UtilMisc.<String, Object>toMap("uomId", codSurchargeCurrencyUomId, "uomIdTo", currencyCode, "originalValue", codSurchargePackageAmount));
                     if (ServiceUtil.isError(convertUomResult)) return convertUomResult;
                     if (convertUomResult.containsKey("convertedValue")) {
-                        codSurchargePackageAmount = ((BigDecimal) convertUomResult.get("convertedValue")).setScale(decimals, rounding);
+                        codSurchargePackageAmount = ((BigDecimal) convertUomResult.get("convertedValue")).setScale(DECIMALS, ROUNDING);
                     }
 
                     // Add the amount of the surcharge for the package, if the surcharge should be on all packages or the first and this is the first package
@@ -598,7 +597,7 @@ public class UpsServices {
                         packageValue = packageValue.add(codSurchargePackageAmount);
                     }
 
-                    UtilXml.addChildElementValue(codAmountElement, "MonetaryValue", packageValue.setScale(decimals, rounding).toString(), shipmentConfirmRequestDoc);
+                    UtilXml.addChildElementValue(codAmountElement, "MonetaryValue", packageValue.setScale(DECIMALS, ROUNDING).toString(), shipmentConfirmRequestDoc);
                 }
             }
             
@@ -773,7 +772,7 @@ public class UpsServices {
                 errorList.add(UtilProperties.getMessage(RES_ERROR, "FacilityShipmentUpsErrorParsingBillingWeight",
                         UtilMisc.toMap("billingWeight", billingWeight, "errorString", e.toString()), locale));
             }
-            shipmentRouteSegment.set("billingWeightUomId", unitsUpsToOfbiz.get(billingWeightUnitOfMeasurement));
+            shipmentRouteSegment.set("billingWeightUomId", UPS_TO_OFBIZ.get(billingWeightUnitOfMeasurement));
 
             // store the ShipmentIdentificationNumber and ShipmentDigest
             String shipmentIdentificationNumber = UtilXml.childElementValue(shipmentConfirmResponseElement, "ShipmentIdentificationNumber");
@@ -1052,7 +1051,7 @@ public class UpsServices {
                 errorList.add(UtilProperties.getMessage(RES_ERROR, "FacilityShipmentUpsErrorParsingBillingWeight",
                         UtilMisc.toMap("billingWeight", billingWeight, "errorString", e.toString()), locale));
             }
-            shipmentRouteSegment.set("billingWeightUomId", unitsUpsToOfbiz.get(billingWeightUnitOfMeasurement));
+            shipmentRouteSegment.set("billingWeightUomId", UPS_TO_OFBIZ.get(billingWeightUnitOfMeasurement));
 
             // store the ShipmentIdentificationNumber and ShipmentDigest
             String shipmentIdentificationNumber = UtilXml.childElementValue(shipmentResultsElement, "ShipmentIdentificationNumber");
@@ -2416,7 +2415,7 @@ public class UpsServices {
 
             // Child of Shipment: ReturnService
             Element returnServiceElement = UtilXml.addChildElement(shipmentElement, "ReturnService", shipmentConfirmRequestDoc);
-            UtilXml.addChildElementValue(returnServiceElement, "Code", String.valueOf(returnServiceCode), shipmentConfirmRequestDoc);
+            UtilXml.addChildElementValue(returnServiceElement, "Code", String.valueOf(RET_SERVICE_CODE), shipmentConfirmRequestDoc);
 
             // Child of Shipment: Shipper
             String shipperNumber = getShipmentGatewayConfigValue(delegator, shipmentGatewayConfigId, "shipperNumber", RESOURCE, "shipment.ups.shipper.number", "");
@@ -2895,14 +2894,14 @@ public class UpsServices {
                         BigDecimal length = (BigDecimal) shipmentPackage.get("boxLength");
                         BigDecimal width = (BigDecimal) shipmentPackage.get("boxWidth");
                         BigDecimal height = (BigDecimal) shipmentPackage.get("boxHeight");
-                        UtilXml.addChildElementValue(dimensionsElement, "Length", length.setScale(decimals, rounding).toString(), rateRequestDoc);
-                        UtilXml.addChildElementValue(dimensionsElement, "Width", width.setScale(decimals, rounding).toString(), rateRequestDoc);
-                        UtilXml.addChildElementValue(dimensionsElement, "Height", height.setScale(decimals, rounding).toString(), rateRequestDoc);
+                        UtilXml.addChildElementValue(dimensionsElement, "Length", length.setScale(DECIMALS, ROUNDING).toString(), rateRequestDoc);
+                        UtilXml.addChildElementValue(dimensionsElement, "Width", width.setScale(DECIMALS, ROUNDING).toString(), rateRequestDoc);
+                        UtilXml.addChildElementValue(dimensionsElement, "Height", height.setScale(DECIMALS, ROUNDING).toString(), rateRequestDoc);
                 }
 
                 Element packageWeightElement = UtilXml.addChildElement(packageElement, "PackageWeight", rateRequestDoc);
                 Element packageWeightUnitOfMeasurementElement = UtilXml.addChildElement(packageElement, "UnitOfMeasurement", rateRequestDoc);
-                String weightUomUps = unitsOfbizToUps.get(shipmentPackage.get("weightUomId"));
+                String weightUomUps = OFBIZ_TO_UPS.get(shipmentPackage.get("weightUomId"));
                 if (weightUomUps != null) {
                     UtilXml.addChildElementValue(packageWeightUnitOfMeasurementElement, "Code", weightUomUps, rateRequestDoc);
                 } else {
diff --git a/applications/product/src/main/java/org/apache/ofbiz/shipment/weightPackage/WeightPackageSession.java b/applications/product/src/main/java/org/apache/ofbiz/shipment/weightPackage/WeightPackageSession.java
index 53ba0db..502ec9c 100644
--- a/applications/product/src/main/java/org/apache/ofbiz/shipment/weightPackage/WeightPackageSession.java
+++ b/applications/product/src/main/java/org/apache/ofbiz/shipment/weightPackage/WeightPackageSession.java
@@ -67,7 +67,7 @@ public class WeightPackageSession implements Serializable {
 
     private transient Delegator _delegator = null;
     private transient LocalDispatcher _dispatcher = null;
-    private static RoundingMode rounding = UtilNumber.getRoundingMode("invoice.rounding");
+    private static final RoundingMode ROUNDING_MODE = UtilNumber.getRoundingMode("invoice.rounding");
 
     public WeightPackageSession() {
     }
@@ -393,7 +393,7 @@ public class WeightPackageSession implements Serializable {
         if (estimatedShipCost.compareTo(BigDecimal.ZERO) == 0) {
             diffInShipCostInPerc = actualShippingCost;
         } else {
-            diffInShipCostInPerc = (((actualShippingCost.subtract(estimatedShipCost)).divide(estimatedShipCost, 2, rounding)).multiply(new BigDecimal(100))).abs();
+            diffInShipCostInPerc = (((actualShippingCost.subtract(estimatedShipCost)).divide(estimatedShipCost, 2, ROUNDING_MODE)).multiply(new BigDecimal(100))).abs();
         }
         return doEstimates.compareTo(diffInShipCostInPerc) == -1;
     }