You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Taher Alkhateeb <sl...@gmail.com> on 2017/12/16 15:24:30 UTC

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

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
<ja...@les7arts.com> 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, <jl...@apache.org> 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) {
>>
>