You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2017/12/16 15:14:08 UTC
Re: svn commit: r1818378 -
/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apac
he/ofbiz/accounting/tax/TaxAuthorityServices.java
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) {
>
Re: svn commit: r1818378 - /ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apac
he/ofbiz/accounting/tax/TaxAuthorityServices.java
Posted by Taher Alkhateeb <sl...@gmail.com>.
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) {
>>
>