Re: svn commit: r1818378 - /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java

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

Re: svn commit: r1818378 - /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java

taher
I think it is always better to respect limit width because it has many
implications that affect other developers.

Anyway, I do not think the right place to make arguments is in commit
messages. A commit message is supposed to hold information on what was
done, not a place to argue formatting conventions or what you'd like to do.
Perhaps it is more appropriate to use the ML for that.

On Dec 16, 2017 12:28 PM, <[hidden email]> wrote:

Author: jleroux
Date: Sat Dec 16 09:28:37 2017
New Revision: 1818378

URL: http://svn.apache.org/viewvc?rev=1818378&view=rev
Log:
No functional change, just trivial formatting

I began to reformat this supposed refactoring commit, but stopped and
decided
to rather discuss this in the related Jira.

BTW automatically cutting lines at some narrow width does not make sense
most of
the time.
See EntityQuery cases for instance, are they not easier to read when, IMO,
rightly formatted, as I did here?

Modified:
    ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
/java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java

Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
/java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/app
lications/accounting/src/main/java/org/apache/ofbiz/accounti
ng/tax/TaxAuthorityServices.java?rev=1818378&r1=1818377&r2=1818378&view=diff
============================================================
==================
--- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
/java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
/java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java Sat Dec 16
09:28:37 2017
@@ -63,8 +63,7 @@ public class TaxAuthorityServices {
     public static final int salestaxRounding =
UtilNumber.getBigDecimalRoundingMode("salestax.rounding");
     public static final String resource = "AccountingUiLabels";

-    public static Map<String, Object>
rateProductTaxCalcForDisplay(DispatchContext
dctx,
-            Map<String, ? extends Object> context) {
+    public static Map<String, Object>
rateProductTaxCalcForDisplay(DispatchContext
dctx, Map<String, ? extends Object> context) {
         Delegator delegator = dctx.getDelegator();
         String productStoreId = (String) context.get("productStoreId");
         String billToPartyId = (String) context.get("billToPartyId");
@@ -87,20 +86,27 @@ public class TaxAuthorityServices {
         }

         try {
-            GenericValue product = EntityQuery.use(delegator).fro
m("Product").where("productId", productId).cache()
+            GenericValue product = EntityQuery.use(delegator)
+                    .from("Product")
+                    .where("productId", productId).cache()
+                    .queryOne();
+            GenericValue productStore = EntityQuery.use(delegator)
+                    .from("ProductStore")
+                    .where("productStoreId", productStoreId)
+                    .cache()
                     .queryOne();
-            GenericValue productStore = EntityQuery.use(delegator).fro
m("ProductStore").where("productStoreId",
-                    productStoreId).cache().queryOne();
             if (productStore == null) {
-                throw new IllegalArgumentException("Could not find
ProductStore with ID [" + productStoreId
-                        + "] for tax calculation");
+                throw new IllegalArgumentException("Could not find
ProductStore with ID [" + productStoreId + "] for tax calculation");
             }

             if ("Y".equals(productStore.getString("showPricesWithVatTax")))
{
                 Set<GenericValue> taxAuthoritySet = new HashSet<>();
                 if (productStore.get("vatTaxAuthPartyId") == null) {
-                    List<GenericValue> taxAuthorityRawList =
EntityQuery.use(delegator).from("TaxAuthority")
-                            .where("taxAuthGeoId",
productStore.get("vatTaxAuthGeoId")).cache().queryList();
+                    List<GenericValue> taxAuthorityRawList =
EntityQuery.use(delegator)
+                            .from("TaxAuthority")
+                            .where("taxAuthGeoId",
productStore.get("vatTaxAuthGeoId"))
+                            .cache()
+                            .queryList();
                     taxAuthoritySet.addAll(taxAuthorityRawList);
                 } else {
                     GenericValue taxAuthority =
EntityQuery.use(delegator).from("TaxAuthority").where("taxAuthGeoId",
@@ -175,7 +181,9 @@ public class TaxAuthorityServices {
         GenericValue facility = null;
         try {
             if (productStoreId != null) {
-                productStore = EntityQuery.use(delegator).fro
m("ProductStore").where("productStoreId", productStoreId)
+                productStore = EntityQuery.use(delegator)
+                        .from("ProductStore")
+                        .where("productStoreId", productStoreId)
                         .queryOne();
             }
             if (facilityId != null) {
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1818378 - /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apac he/ofbiz/accounting/tax/TaxAuthorityServices.java

Jacques Le Roux
Administrator
Actually we had a good discussion in the related Jira https://issues.apache.org/jira/browse/OFBIZ-9877

We agreed it's time to rediscuss this again and I agree/and-suggested-there the dev ML is the best place for that

Jacques


Le 16/12/2017 à 14:27, Taher Alkhateeb a écrit :

> I think it is always better to respect limit width because it has many
> implications that affect other developers.
>
> Anyway, I do not think the right place to make arguments is in commit
> messages. A commit message is supposed to hold information on what was
> done, not a place to argue formatting conventions or what you'd like to do.
> Perhaps it is more appropriate to use the ML for that.
>
> On Dec 16, 2017 12:28 PM, <[hidden email]> wrote:
>
> Author: jleroux
> Date: Sat Dec 16 09:28:37 2017
> New Revision: 1818378
>
> URL: http://svn.apache.org/viewvc?rev=1818378&view=rev
> Log:
> No functional change, just trivial formatting
>
> I began to reformat this supposed refactoring commit, but stopped and
> decided
> to rather discuss this in the related Jira.
>
> BTW automatically cutting lines at some narrow width does not make sense
> most of
> the time.
> See EntityQuery cases for instance, are they not easier to read when, IMO,
> rightly formatted, as I did here?
>
> Modified:
>      ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java
>
> Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/app
> lications/accounting/src/main/java/org/apache/ofbiz/accounti
> ng/tax/TaxAuthorityServices.java?rev=1818378&r1=1818377&r2=1818378&view=diff
> ============================================================
> ==================
> --- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java (original)
> +++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java Sat Dec 16
> 09:28:37 2017
> @@ -63,8 +63,7 @@ public class TaxAuthorityServices {
>       public static final int salestaxRounding =
> UtilNumber.getBigDecimalRoundingMode("salestax.rounding");
>       public static final String resource = "AccountingUiLabels";
>
> -    public static Map<String, Object>
> rateProductTaxCalcForDisplay(DispatchContext
> dctx,
> -            Map<String, ? extends Object> context) {
> +    public static Map<String, Object>
> rateProductTaxCalcForDisplay(DispatchContext
> dctx, Map<String, ? extends Object> context) {
>           Delegator delegator = dctx.getDelegator();
>           String productStoreId = (String) context.get("productStoreId");
>           String billToPartyId = (String) context.get("billToPartyId");
> @@ -87,20 +86,27 @@ public class TaxAuthorityServices {
>           }
>
>           try {
> -            GenericValue product = EntityQuery.use(delegator).fro
> m("Product").where("productId", productId).cache()
> +            GenericValue product = EntityQuery.use(delegator)
> +                    .from("Product")
> +                    .where("productId", productId).cache()
> +                    .queryOne();
> +            GenericValue productStore = EntityQuery.use(delegator)
> +                    .from("ProductStore")
> +                    .where("productStoreId", productStoreId)
> +                    .cache()
>                       .queryOne();
> -            GenericValue productStore = EntityQuery.use(delegator).fro
> m("ProductStore").where("productStoreId",
> -                    productStoreId).cache().queryOne();
>               if (productStore == null) {
> -                throw new IllegalArgumentException("Could not find
> ProductStore with ID [" + productStoreId
> -                        + "] for tax calculation");
> +                throw new IllegalArgumentException("Could not find
> ProductStore with ID [" + productStoreId + "] for tax calculation");
>               }
>
>               if ("Y".equals(productStore.getString("showPricesWithVatTax")))
> {
>                   Set<GenericValue> taxAuthoritySet = new HashSet<>();
>                   if (productStore.get("vatTaxAuthPartyId") == null) {
> -                    List<GenericValue> taxAuthorityRawList =
> EntityQuery.use(delegator).from("TaxAuthority")
> -                            .where("taxAuthGeoId",
> productStore.get("vatTaxAuthGeoId")).cache().queryList();
> +                    List<GenericValue> taxAuthorityRawList =
> EntityQuery.use(delegator)
> +                            .from("TaxAuthority")
> +                            .where("taxAuthGeoId",
> productStore.get("vatTaxAuthGeoId"))
> +                            .cache()
> +                            .queryList();
>                       taxAuthoritySet.addAll(taxAuthorityRawList);
>                   } else {
>                       GenericValue taxAuthority =
> EntityQuery.use(delegator).from("TaxAuthority").where("taxAuthGeoId",
> @@ -175,7 +181,9 @@ public class TaxAuthorityServices {
>           GenericValue facility = null;
>           try {
>               if (productStoreId != null) {
> -                productStore = EntityQuery.use(delegator).fro
> m("ProductStore").where("productStoreId", productStoreId)
> +                productStore = EntityQuery.use(delegator)
> +                        .from("ProductStore")
> +                        .where("productStoreId", productStoreId)
>                           .queryOne();
>               }
>               if (facilityId != null) {
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1818378 - /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apac he/ofbiz/accounting/tax/TaxAuthorityServices.java

taher
I see, well I suppose perhaps JIRA is also not the best place for
discussing "general" standards and coding practices. The ML is usually
the first platform for these discussions.

I also noted that Michael had concerns with the _value_ of the maximum
width, not whether we should or should not have one (I could be
mistaken, so apology in advance if I misread). I think either way it
is worth a good discussion because even if 80 characters is too short
let's say, then 800 characters might be too long! It's good to have a
standard which unifies us I think.

On Sat, Dec 16, 2017 at 6:14 PM, Jacques Le Roux
<[hidden email]> wrote:

> Actually we had a good discussion in the related Jira
> https://issues.apache.org/jira/browse/OFBIZ-9877
>
> We agreed it's time to rediscuss this again and I agree/and-suggested-there
> the dev ML is the best place for that
>
> Jacques
>
>
>
> Le 16/12/2017 à 14:27, Taher Alkhateeb a écrit :
>>
>> I think it is always better to respect limit width because it has many
>> implications that affect other developers.
>>
>> Anyway, I do not think the right place to make arguments is in commit
>> messages. A commit message is supposed to hold information on what was
>> done, not a place to argue formatting conventions or what you'd like to
>> do.
>> Perhaps it is more appropriate to use the ML for that.
>>
>> On Dec 16, 2017 12:28 PM, <[hidden email]> wrote:
>>
>> Author: jleroux
>> Date: Sat Dec 16 09:28:37 2017
>> New Revision: 1818378
>>
>> URL: http://svn.apache.org/viewvc?rev=1818378&view=rev
>> Log:
>> No functional change, just trivial formatting
>>
>> I began to reformat this supposed refactoring commit, but stopped and
>> decided
>> to rather discuss this in the related Jira.
>>
>> BTW automatically cutting lines at some narrow width does not make sense
>> most of
>> the time.
>> See EntityQuery cases for instance, are they not easier to read when, IMO,
>> rightly formatted, as I did here?
>>
>> Modified:
>>      ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
>> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java
>>
>> Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
>> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/app
>> lications/accounting/src/main/java/org/apache/ofbiz/accounti
>>
>> ng/tax/TaxAuthorityServices.java?rev=1818378&r1=1818377&r2=1818378&view=diff
>> ============================================================
>> ==================
>> --- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
>> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java (original)
>> +++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main
>> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java Sat Dec 16
>> 09:28:37 2017
>> @@ -63,8 +63,7 @@ public class TaxAuthorityServices {
>>       public static final int salestaxRounding =
>> UtilNumber.getBigDecimalRoundingMode("salestax.rounding");
>>       public static final String resource = "AccountingUiLabels";
>>
>> -    public static Map<String, Object>
>> rateProductTaxCalcForDisplay(DispatchContext
>> dctx,
>> -            Map<String, ? extends Object> context) {
>> +    public static Map<String, Object>
>> rateProductTaxCalcForDisplay(DispatchContext
>> dctx, Map<String, ? extends Object> context) {
>>           Delegator delegator = dctx.getDelegator();
>>           String productStoreId = (String) context.get("productStoreId");
>>           String billToPartyId = (String) context.get("billToPartyId");
>> @@ -87,20 +86,27 @@ public class TaxAuthorityServices {
>>           }
>>
>>           try {
>> -            GenericValue product = EntityQuery.use(delegator).fro
>> m("Product").where("productId", productId).cache()
>> +            GenericValue product = EntityQuery.use(delegator)
>> +                    .from("Product")
>> +                    .where("productId", productId).cache()
>> +                    .queryOne();
>> +            GenericValue productStore = EntityQuery.use(delegator)
>> +                    .from("ProductStore")
>> +                    .where("productStoreId", productStoreId)
>> +                    .cache()
>>                       .queryOne();
>> -            GenericValue productStore = EntityQuery.use(delegator).fro
>> m("ProductStore").where("productStoreId",
>> -                    productStoreId).cache().queryOne();
>>               if (productStore == null) {
>> -                throw new IllegalArgumentException("Could not find
>> ProductStore with ID [" + productStoreId
>> -                        + "] for tax calculation");
>> +                throw new IllegalArgumentException("Could not find
>> ProductStore with ID [" + productStoreId + "] for tax calculation");
>>               }
>>
>>               if
>> ("Y".equals(productStore.getString("showPricesWithVatTax")))
>> {
>>                   Set<GenericValue> taxAuthoritySet = new HashSet<>();
>>                   if (productStore.get("vatTaxAuthPartyId") == null) {
>> -                    List<GenericValue> taxAuthorityRawList =
>> EntityQuery.use(delegator).from("TaxAuthority")
>> -                            .where("taxAuthGeoId",
>> productStore.get("vatTaxAuthGeoId")).cache().queryList();
>> +                    List<GenericValue> taxAuthorityRawList =
>> EntityQuery.use(delegator)
>> +                            .from("TaxAuthority")
>> +                            .where("taxAuthGeoId",
>> productStore.get("vatTaxAuthGeoId"))
>> +                            .cache()
>> +                            .queryList();
>>                       taxAuthoritySet.addAll(taxAuthorityRawList);
>>                   } else {
>>                       GenericValue taxAuthority =
>> EntityQuery.use(delegator).from("TaxAuthority").where("taxAuthGeoId",
>> @@ -175,7 +181,9 @@ public class TaxAuthorityServices {
>>           GenericValue facility = null;
>>           try {
>>               if (productStoreId != null) {
>> -                productStore = EntityQuery.use(delegator).fro
>> m("ProductStore").where("productStoreId", productStoreId)
>> +                productStore = EntityQuery.use(delegator)
>> +                        .from("ProductStore")
>> +                        .where("productStoreId", productStoreId)
>>                           .queryOne();
>>               }
>>               if (facilityId != null) {
>>
>