Re: svn commit: r1343472 - in /ofbiz/trunk/applications: accounting/src/org/ofbiz/accounting/invoice/ manufacturing/src/org/ofbiz/manufacturing/bom/ manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ order/src/org/ofbiz/order/order/ order/src/org/ofbiz/

[Jacques Le Roux]

Hi Adam,

I don't know what others think about that, but personnally I don't like it. For instance we get now from beginning of the line
(trailers blanks included)

'                                            GenericValue refurbItemAssoc =
EntityUtil.getFirst(EntityUtil.filterByDate(product.getRelated("MainProductAssoc", UtilMisc.toMap("productAssocTypeId",
"PRODUCT_REFURB"),'
214 chars
'                        repsCustomers =
EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship", UtilMisc.toMap("roleTypeIdFrom",
"AGENT", "roleTypeIdTo", "CUSTOMER", "partyIdTo", partyId)));'
232 chars
'                            repsCustomers =
EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship", UtilMisc.toMap("roleTypeIdFrom",
"SALES_REP", "roleTypeIdTo", "CUSTOMER", "partyIdTo", partyId)));
241 chars

'                        List<GenericValue> productAssocs = EntityUtil.filterByDate(product.getRelatedCache("MainProductAssoc",
UtilMisc.toMap("productAssocTypeId", "PRODUCT_VARIANT"), UtilMisc.toList("sequenceNum")));'
218 chars

'              <#assign itemProductAssocList = cartLine.getProduct().getRelated("MainProductAssoc",
Static["org.ofbiz.base.util.UtilMisc"].toList("productAssocTypeId", "sequenceNum"))?if_exists/>'
195 chars
'                                List<GenericValue> variantProductFeatureAndAppls = variant.getRelated("ProductFeatureAndAppl",
UtilMisc.toMap("productFeatureTypeId", productFeatureTypeId, "productFeatureApplTypeId", "STANDARD_FEATURE", "description",
description), null);'
271 chars
'         if (UtilValidate.isNotEmpty(instanceProduct) && EntityTypeUtil.hasParentType(delegator, "ProductType", "productTypeId",
instanceProduct.getString("productTypeId"), "parentTypeId", "AGGREGATED")) {
205 chars

I don't know which screen size you use, but for me those lines are too long.

Some time ago, I suggested that we could get a consensus on the maximum code lines length. Eclipse (and I guess most IDEs) is able
then to format/split/wrap the lines on this basis.
Rules and best practices are sometimes preventing creativity, but I don't think in this case, it's a problem.

Personaly I think that around 180 shoud be a maximum. I have attached My Java code formatter at the
https://cwiki.apache.org/confluence/display/OFBADMIN/Coding+Conventions page as an example.

Jacques


From: <[hidden email]>

> Author: doogie
> Date: Tue May 29 04:10:11 2012
> New Revision: 1343472
>
> URL: http://svn.apache.org/viewvc?rev=1343472&view=rev
> Log:
> OPTIMIZE: Join a few overly long split lines.
>
> Modified:
>    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceServices.java
>    ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceWorker.java
>    ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/bom/BOMTree.java
>    ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRun.java
>    ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunServices.java
>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java
>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java
>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
>    ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
>    ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>    ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
>    ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductServices.java
>    ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductWorker.java
>    ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
>
> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceServices.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceServices.java (original)
> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceServices.java Tue May 29 04:10:11 2012
> @@ -333,8 +333,7 @@ public class InvoiceServices {
>                 // for purchase orders, the pay to address is the BILLING_LOCATION of the vendor
>                 GenericValue billFromVendor = orh.getPartyFromRole("BILL_FROM_VENDOR");
>                 if (billFromVendor != null) {
> -                    List<GenericValue> billingContactMechs =
> billFromVendor.getRelatedOne("Party").getRelatedByAnd("PartyContactMechPurpose",
> -                            UtilMisc.toMap("contactMechPurposeTypeId", "BILLING_LOCATION"));
> +                    List<GenericValue> billingContactMechs =
> billFromVendor.getRelatedOne("Party").getRelatedByAnd("PartyContactMechPurpose", UtilMisc.toMap("contactMechPurposeTypeId",
> "BILLING_LOCATION"));
>                     if (UtilValidate.isNotEmpty(billingContactMechs)) {
>                         payToAddress = EntityUtil.getFirst(billingContactMechs);
>                     }
>
> Modified: ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceWorker.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceWorker.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceWorker.java (original)
> +++ ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceWorker.java Tue May 29 04:10:11 2012
> @@ -201,8 +201,7 @@ public class InvoiceWorker {
>         // remaining code is the old method, which we leave here for compatibility purposes
>         List<GenericValue> billToRoles = null;
>         try {
> -            billToRoles = invoice.getRelated("InvoiceRole", UtilMisc.toMap("roleTypeId", "BILL_TO_CUSTOMER"),
> -                UtilMisc.toList("-datetimePerformed"));
> +            billToRoles = invoice.getRelated("InvoiceRole", UtilMisc.toMap("roleTypeId", "BILL_TO_CUSTOMER"),
> UtilMisc.toList("-datetimePerformed"));
>         } catch (GenericEntityException e) {
>             Debug.logError(e, "Trouble getting InvoiceRole list", module);
>         }
> @@ -245,8 +244,7 @@ public class InvoiceWorker {
>         // remaining code is the old method, which we leave here for compatibility purposes
>         List<GenericValue> sendFromRoles = null;
>         try {
> -            sendFromRoles = invoice.getRelated("InvoiceRole", UtilMisc.toMap("roleTypeId", "BILL_FROM_VENDOR"),
> -                UtilMisc.toList("-datetimePerformed"));
> +            sendFromRoles = invoice.getRelated("InvoiceRole", UtilMisc.toMap("roleTypeId", "BILL_FROM_VENDOR"),
> UtilMisc.toList("-datetimePerformed"));
>         } catch (GenericEntityException e) {
>             Debug.logError(e, "Trouble getting InvoiceRole list", module);
>         }
>
> Modified: ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/bom/BOMTree.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/bom/BOMTree.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/bom/BOMTree.java (original)
> +++ ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/bom/BOMTree.java Tue May 29 04:10:11 2012
> @@ -174,8 +174,7 @@ public class BOMTree {
>     }
>
>     private boolean hasBom(GenericValue product, Date inDate) throws GenericEntityException {
> -        List<GenericValue> children = product.getRelatedByAnd("MainProductAssoc",
> -                UtilMisc.toMap("productAssocTypeId", bomTypeId));
> +        List<GenericValue> children = product.getRelatedByAnd("MainProductAssoc", UtilMisc.toMap("productAssocTypeId",
> bomTypeId));
>         children = EntityUtil.filterByDate(children, inDate);
>         return UtilValidate.isNotEmpty(children);
>     }
>
> Modified: ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRun.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRun.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRun.java (original)
> +++ ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRun.java Tue May 29 04:10:11 2012
> @@ -165,8 +165,7 @@ public class ProductionRun {
>         if (exist()) {
>             if (productProduced == null) {
>                 try {
> -                    List<GenericValue> productionRunProducts = productionRun.getRelated("WorkEffortGoodStandard",
> -                            UtilMisc.toMap("workEffortGoodStdTypeId", "PRUN_PROD_DELIV"), null);
> +                    List<GenericValue> productionRunProducts = productionRun.getRelated("WorkEffortGoodStandard",
> UtilMisc.toMap("workEffortGoodStdTypeId", "PRUN_PROD_DELIV"), null);
>                     this.productionRunProduct = EntityUtil.getFirst(productionRunProducts);
>                     quantity = productionRunProduct.getBigDecimal("estimatedQuantity");
>                     productProduced = productionRunProduct.getRelatedOneCache("Product");
>
> Modified: ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunServices.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunServices.java (original)
> +++ ofbiz/trunk/applications/manufacturing/src/org/ofbiz/manufacturing/jobshopmgt/ProductionRunServices.java Tue May 29 04:10:11
> 2012
> @@ -1109,8 +1109,7 @@ public class ProductionRunServices {
>                 List<GenericValue> setupCosts = fixedAsset.getRelatedByAnd("FixedAssetStdCost",
>                         UtilMisc.toMap("fixedAssetStdCostTypeId", "SETUP_COST"));
>                 GenericValue setupCost = EntityUtil.getFirst(EntityUtil.filterByDate(setupCosts));
> -                List<GenericValue> usageCosts = fixedAsset.getRelatedByAnd("FixedAssetStdCost",
> -                        UtilMisc.toMap("fixedAssetStdCostTypeId", "USAGE_COST"));
> +                List<GenericValue> usageCosts = fixedAsset.getRelatedByAnd("FixedAssetStdCost",
> UtilMisc.toMap("fixedAssetStdCostTypeId", "USAGE_COST"));
>                 GenericValue usageCost = EntityUtil.getFirst(EntityUtil.filterByDate(usageCosts));
>                 if (UtilValidate.isNotEmpty(setupCost) || UtilValidate.isNotEmpty(usageCost)) {
>                     String currencyUomId = (setupCost != null? setupCost.getString("amountUomId"):
> usageCost.getString("amountUomId"));
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java Tue May 29 04:10:11 2012
> @@ -466,8 +466,7 @@ public class OrderReadHelper {
>     @Deprecated
>     public GenericValue getShippingAddress() {
>         try {
> -            GenericValue orderContactMech = EntityUtil.getFirst(orderHeader.getRelatedByAnd("OrderContactMech", UtilMisc.toMap(
> -                            "contactMechPurposeTypeId", "SHIPPING_LOCATION")));
> +            GenericValue orderContactMech = EntityUtil.getFirst(orderHeader.getRelatedByAnd("OrderContactMech",
> UtilMisc.toMap("contactMechPurposeTypeId", "SHIPPING_LOCATION")));
>
>             if (orderContactMech != null) {
>                 GenericValue contactMech = orderContactMech.getRelatedOne("ContactMech");
> @@ -558,8 +557,7 @@ public class OrderReadHelper {
>
>     public List<GenericValue> getOrderContactMechs(String purposeTypeId) {
>         try {
> -            return orderHeader.getRelatedByAnd("OrderContactMech",
> -                    UtilMisc.toMap("contactMechPurposeTypeId", purposeTypeId));
> +            return orderHeader.getRelatedByAnd("OrderContactMech", UtilMisc.toMap("contactMechPurposeTypeId", purposeTypeId));
>         } catch (GenericEntityException e) {
>             Debug.logWarning(e, module);
>         }
> @@ -2669,8 +2667,7 @@ public class OrderReadHelper {
>         } else if (security.hasEntityPermission("ORDERMGR", "_ROLEVIEW", userLogin)) {
>             List<GenericValue> orderRoles = null;
>             try {
> -                orderRoles = orderHeader.getRelatedByAnd("OrderRole",
> -                        UtilMisc.toMap("partyId", userLogin.getString("partyId")));
> +                orderRoles = orderHeader.getRelatedByAnd("OrderRole", UtilMisc.toMap("partyId", userLogin.getString("partyId")));
>             } catch (GenericEntityException e) {
>                 Debug.logError(e, "Cannot get OrderRole from OrderHeader", module);
>             }
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java Tue May 29 04:10:11 2012
> @@ -1825,9 +1825,7 @@ public class OrderReturnServices {
>                                 if ("CUSTOMER_RETURN".equals(returnHeaderTypeId)) {
>                                     try {
>                                         if (UtilValidate.isNotEmpty(product)) {
> -                                            GenericValue refurbItemAssoc =
> EntityUtil.getFirst(EntityUtil.filterByDate(product.getRelated("MainProductAssoc",
> -
> UtilMisc.toMap("productAssocTypeId", "PRODUCT_REFURB"),
> -
> UtilMisc.toList("sequenceNum"))));
> +                                            GenericValue refurbItemAssoc =
> EntityUtil.getFirst(EntityUtil.filterByDate(product.getRelated("MainProductAssoc", UtilMisc.toMap("productAssocTypeId",
> "PRODUCT_REFURB"), UtilMisc.toList("sequenceNum"))));
>                                             if (UtilValidate.isNotEmpty(refurbItemAssoc)) {
>                                                 refurbItem = refurbItemAssoc.getRelatedOne("AssocProduct");
>                                             }
> @@ -1910,8 +1908,7 @@ public class OrderReturnServices {
>                                     List<GenericValue> repairItems = null;
>                                     try {
>                                         if (UtilValidate.isNotEmpty(product)) {
> -                                            repairItems = EntityUtil.filterByDate(product.getRelated("MainProductAssoc",
> -                                                    UtilMisc.toMap("productAssocTypeId", "PRODUCT_REPAIR_SRV"),
> UtilMisc.toList("sequenceNum")));
> +                                            repairItems = EntityUtil.filterByDate(product.getRelated("MainProductAssoc",
> UtilMisc.toMap("productAssocTypeId", "PRODUCT_REPAIR_SRV"), UtilMisc.toList("sequenceNum")));
>                                         }
>                                     } catch (GenericEntityException e) {
>                                         Debug.logError(e, module);
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java Tue May 29 04:10:11 2012
> @@ -152,8 +152,7 @@ public class OrderServices {
>                     // check sales agent/customer relationship
>                     List<GenericValue> repsCustomers = new LinkedList<GenericValue>();
>                     try {
> -                        repsCustomers =
> EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship",
> -                                UtilMisc.toMap("roleTypeIdFrom", "AGENT", "roleTypeIdTo", "CUSTOMER", "partyIdTo", partyId)));
> +                        repsCustomers =
> EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship", UtilMisc.toMap("roleTypeIdFrom",
> "AGENT", "roleTypeIdTo", "CUSTOMER", "partyIdTo", partyId)));
>                     } catch (GenericEntityException ex) {
>                         Debug.logError("Could not determine if " + partyId + " is a customer of user " +
> userLogin.getString("userLoginId") + " due to " + ex.getMessage(), module);
>                     }
> @@ -163,8 +162,7 @@ public class OrderServices {
>                     if (!hasPermission) {
>                         // check sales sales rep/customer relationship
>                         try {
> -                            repsCustomers =
> EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship",
> -                                    UtilMisc.toMap("roleTypeIdFrom", "SALES_REP", "roleTypeIdTo", "CUSTOMER", "partyIdTo",
> partyId)));
> +                            repsCustomers =
> EntityUtil.filterByDate(userLogin.getRelatedOne("Party").getRelatedByAnd("FromPartyRelationship", UtilMisc.toMap("roleTypeIdFrom",
> "SALES_REP", "roleTypeIdTo", "CUSTOMER", "partyIdTo", partyId)));
>                         } catch (GenericEntityException ex) {
>                             Debug.logError("Could not determine if " + partyId + " is a customer of user " +
> userLogin.getString("userLoginId") + " due to " + ex.getMessage(), module);
>                         }
> @@ -2866,8 +2864,7 @@ public class OrderServices {
>         }
>         for (int i = 0; i < purpose.length; i++) {
>             try {
> -                GenericValue orderContactMech = EntityUtil.getFirst(orderHeader.getRelatedByAnd("OrderContactMech",
> -                            UtilMisc.toMap("contactMechPurposeTypeId", purpose[i])));
> +                GenericValue orderContactMech = EntityUtil.getFirst(orderHeader.getRelatedByAnd("OrderContactMech",
> UtilMisc.toMap("contactMechPurposeTypeId", purpose[i])));
>                 GenericValue contactMech = orderContactMech.getRelatedOne("ContactMech");
>
>                 if (contactMech != null) {
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java Tue May 29 04:10:11 2012
> @@ -1423,8 +1423,7 @@ public class ProductPromoWorker {
>                         throw new CartItemModifyException(errMsg);
>                     }
>                     if ("Y".equals(product.getString("isVirtual"))) {
> -                        List<GenericValue> productAssocs = EntityUtil.filterByDate(product.getRelatedCache("MainProductAssoc",
> -                                UtilMisc.toMap("productAssocTypeId", "PRODUCT_VARIANT"), UtilMisc.toList("sequenceNum")));
> +                        List<GenericValue> productAssocs = EntityUtil.filterByDate(product.getRelatedCache("MainProductAssoc",
> UtilMisc.toMap("productAssocTypeId", "PRODUCT_VARIANT"), UtilMisc.toList("sequenceNum")));
>                         for(GenericValue productAssoc : productAssocs) {
>                             optionProductIds.add(productAssoc.getString("productIdTo"));
>                         }
>
> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl (original)
> +++ ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl Tue May 29 04:10:11 2012
> @@ -212,8 +212,7 @@ under the License.
>
>             <#-- Show Associated Products (not for Variants) -->
>             <#if cartLine.getProductId()?exists>
> -              <#assign itemProductAssocList = cartLine.getProduct().getRelated("MainProductAssoc",
> -                  Static["org.ofbiz.base.util.UtilMisc"].toList("productAssocTypeId", "sequenceNum"))?if_exists/>
> +              <#assign itemProductAssocList = cartLine.getProduct().getRelated("MainProductAssoc",
> Static["org.ofbiz.base.util.UtilMisc"].toList("productAssocTypeId", "sequenceNum"))?if_exists/>
>             </#if>
>             <#if itemProductAssocList?exists && itemProductAssocList?has_content>
>               <tr><td colspan="8"><hr /></td></tr>
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductEvents.java Tue May 29 04:10:11 2012
> @@ -691,9 +691,7 @@ public class ProductEvents {
>                             while (!foundFeatureOnVariant && variantIter.hasNext()) {
>                                 GenericValue variant = variantIter.next();
>                                 // get the selectable features for the variant
> -                                List<GenericValue> variantProductFeatureAndAppls = variant.getRelated("ProductFeatureAndAppl",
> -                                        UtilMisc.toMap("productFeatureTypeId", productFeatureTypeId,
> -                                                "productFeatureApplTypeId", "STANDARD_FEATURE", "description", description),
> null);
> +                                List<GenericValue> variantProductFeatureAndAppls = variant.getRelated("ProductFeatureAndAppl",
> UtilMisc.toMap("productFeatureTypeId", productFeatureTypeId, "productFeatureApplTypeId", "STANDARD_FEATURE", "description",
> description), null);
>                                 if (variantProductFeatureAndAppls.size() > 0) {
>                                     foundFeatureOnVariant = true;
>                                 }
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductServices.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductServices.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductServices.java Tue May 29 04:10:11 2012
> @@ -413,8 +413,7 @@ public class ProductServices {
>             GenericValue mainProduct = product;
>
>             if (product.get("isVariant") != null && product.getString("isVariant").equalsIgnoreCase("Y")) {
> -                List<GenericValue> c = product.getRelatedByAndCache("AssocProductAssoc",
> -                        UtilMisc.toMap("productAssocTypeId", "PRODUCT_VARIANT"));
> +                List<GenericValue> c = product.getRelatedByAndCache("AssocProductAssoc", UtilMisc.toMap("productAssocTypeId",
> "PRODUCT_VARIANT"));
>                 //if (Debug.infoOn()) Debug.logInfo("Found related: " + c, module);
>                 c = EntityUtil.filterByDate(c);
>                 //if (Debug.infoOn()) Debug.logInfo("Found Filtered related: " + c, module);
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductWorker.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductWorker.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductWorker.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductWorker.java Tue May 29 04:10:11 2012
> @@ -150,8 +150,7 @@ public class ProductWorker {
>         GenericValue instanceProduct = delegator.findOne("Product", UtilMisc.toMap("productId", instanceProductId), false);
>
>         if (UtilValidate.isNotEmpty(instanceProduct) && EntityTypeUtil.hasParentType(delegator, "ProductType", "productTypeId",
> instanceProduct.getString("productTypeId"), "parentTypeId", "AGGREGATED")) {
> -            GenericValue productAssoc =
> EntityUtil.getFirst(EntityUtil.filterByDate(instanceProduct.getRelatedByAnd("AssocProductAssoc",
> -                    UtilMisc.toMap("productAssocTypeId", "PRODUCT_CONF"))));
> +            GenericValue productAssoc =
> EntityUtil.getFirst(EntityUtil.filterByDate(instanceProduct.getRelatedByAnd("AssocProductAssoc",
> UtilMisc.toMap("productAssocTypeId", "PRODUCT_CONF"))));
>             if (UtilValidate.isNotEmpty(productAssoc)) {
>                 return productAssoc.getString("productId");
>             }
> @@ -176,8 +175,7 @@ public class ProductWorker {
>         GenericValue aggregatedProduct = delegator.findOne("Product", UtilMisc.toMap("productId", aggregatedProductId), false);
>
>         if (UtilValidate.isNotEmpty(aggregatedProduct) && ("AGGREGATED".equals(aggregatedProduct.getString("productTypeId")) ||
> "AGGREGATED_SERVICE".equals(aggregatedProduct.getString("productTypeId")))) {
> -            List<GenericValue> productAssocs = EntityUtil.filterByDate(aggregatedProduct.getRelatedByAnd("MainProductAssoc",
> -                    UtilMisc.toMap("productAssocTypeId", "PRODUCT_CONF")));
> +            List<GenericValue> productAssocs = EntityUtil.filterByDate(aggregatedProduct.getRelatedByAnd("MainProductAssoc",
> UtilMisc.toMap("productAssocTypeId", "PRODUCT_CONF")));
>             return productAssocs;
>         }
>         return null;
> @@ -198,8 +196,7 @@ public class ProductWorker {
>
>     public static List<GenericValue> getVariantVirtualAssocs(GenericValue variantProduct) throws GenericEntityException {
>         if (variantProduct != null && "Y".equals(variantProduct.getString("isVariant"))) {
> -            List<GenericValue> productAssocs = EntityUtil.filterByDate(variantProduct.getRelatedByAndCache("AssocProductAssoc",
> -                    UtilMisc.toMap("productAssocTypeId", "PRODUCT_VARIANT")));
> +            List<GenericValue> productAssocs = EntityUtil.filterByDate(variantProduct.getRelatedByAndCache("AssocProductAssoc",
> UtilMisc.toMap("productAssocTypeId", "PRODUCT_VARIANT")));
>             return productAssocs;
>         }
>         return null;
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java?rev=1343472&r1=1343471&r2=1343472&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java Tue May 29 04:10:11 2012
> @@ -1005,8 +1005,7 @@ public class UspsServices {
>             }
>
>             // get the packages for this shipment route segment
> -            List<GenericValue> shipmentPackageRouteSegList = shipmentRouteSegment.getRelated("ShipmentPackageRouteSeg", null,
> -                    UtilMisc.toList("+shipmentPackageSeqId"));
> +            List<GenericValue> shipmentPackageRouteSegList = shipmentRouteSegment.getRelated("ShipmentPackageRouteSeg", null,
> UtilMisc.toList("+shipmentPackageSeqId"));
>             if (UtilValidate.isEmpty(shipmentPackageRouteSegList)) {
>                 return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>                         "FacilityShipmentPackageRouteSegsNotFound",
> @@ -1092,8 +1091,7 @@ public class UspsServices {
>                 // Container element
>                 GenericValue carrierShipmentBoxType = null;
>                 List<GenericValue> carrierShipmentBoxTypes = null;
> -                carrierShipmentBoxTypes = shipmentPackage.getRelated("CarrierShipmentBoxType",
> -                        UtilMisc.toMap("partyId", "USPS"), null);
> +                carrierShipmentBoxTypes = shipmentPackage.getRelated("CarrierShipmentBoxType", UtilMisc.toMap("partyId", "USPS"),
> null);
>
>                 if (carrierShipmentBoxTypes.size() > 0) {
>                     carrierShipmentBoxType = carrierShipmentBoxTypes.get(0);
> @@ -1301,8 +1299,7 @@ public class UspsServices {
>             }
>
>             // get the packages for this shipment route segment
> -            List<GenericValue> shipmentPackageRouteSegList = shipmentRouteSegment.getRelated("ShipmentPackageRouteSeg", null,
> -                    UtilMisc.toList("+shipmentPackageSeqId"));
> +            List<GenericValue> shipmentPackageRouteSegList = shipmentRouteSegment.getRelated("ShipmentPackageRouteSeg", null,
> UtilMisc.toList("+shipmentPackageSeqId"));
>             if (UtilValidate.isEmpty(shipmentPackageRouteSegList)) {
>                 return ServiceUtil.returnError(UtilProperties.getMessage(resourceError,
>                         "FacilityShipmentPackageRouteSegsNotFound",
> @@ -1448,8 +1445,7 @@ public class UspsServices {
>             GenericValue shipmentRouteSegment = delegator.findOne("ShipmentRouteSegment",
>                     UtilMisc.toMap("shipmentId", shipmentId, "shipmentRouteSegmentId", shipmentRouteSegmentId), false);
>
> -            List<GenericValue> shipmentPackageRouteSegList = shipmentRouteSegment.getRelated("ShipmentPackageRouteSeg", null,
> -                    UtilMisc.toList("+shipmentPackageSeqId"));
> +            List<GenericValue> shipmentPackageRouteSegList = shipmentRouteSegment.getRelated("ShipmentPackageRouteSeg", null,
> UtilMisc.toList("+shipmentPackageSeqId"));
>
>             for (GenericValue shipmentPackageRouteSeg: shipmentPackageRouteSegList) {
>                 byte[] labelImageBytes = shipmentPackageRouteSeg.getBytes("labelImage");
>
>

[Adam Heath-2]

On 06/02/2012 11:51 AM, Jacques Le Roux wrote:
> Hi Adam,
>
> I don't know what others think about that, but personnally I don't like
> it. For instance we get now from beginning of the line
> (trailers blanks included)

The terminal on my laptop is 198 chars.  At work it's 147.  But that
doesn't matter.

I only combined lines that were affected by the deprecation that I've
been doing.  I didn't do it to all files.

Also, in several of those files, they were not self-consistent.  Some
times, even in the same method, and in the same general area of the
method, some lines were split, and some were all on a single line.  And
the split wasn't consistent.

There is no way that we can ever enforce a single, unified width.
Screen sizes change, font sizes change.  It's too political to come up
with a solution.

So, the only fallback is to never split.  Then it is up to each
end-developer to modify their display to handle the long lines.

ps: This is similiar to svn storing files in the repo with one kind of
line-ending, then upon checkout(or when checking-in), it converts
automatically.

ppd: This is also similiar to keyword expansion; it happens at checkout
time, but the repo stores it unmodified.

[Jacopo Cappellato-4]

On Jun 2, 2012, at 7:15 PM, Adam Heath wrote:

> There is no way that we can ever enforce a single, unified width. Screen sizes change, font sizes change.  It's too political to come up with a solution.
>
> So, the only fallback is to never split.  Then it is up to each end-developer to modify their display to handle the long lines.

I would instead prefer the following guideline: "There is no upper limit to line width; only split long lines if it increases readability of the code".

Jacopo

[Adam Heath-2]

On 06/02/2012 01:06 PM, Jacopo Cappellato wrote:
>
> On Jun 2, 2012, at 7:15 PM, Adam Heath wrote:
>
>> There is no way that we can ever enforce a single, unified width. Screen sizes change, font sizes change.  It's too political to come up with a solution.
>>
>> So, the only fallback is to never split.  Then it is up to each end-developer to modify their display to handle the long lines.
>
> I would instead prefer the following guideline: "There is no upper limit to line width; only split long lines if it increases readability of the code".

That's fine too.  I do the same, when embedding a toMap-type call in the
middle of a method invocation, and I forsee the need to add/remove
fields in the future.

In the commit you commented on, I believe I only combined lines that
seemed to be arbitrarily split on individual method arguments, due to
some random screen width.  It never really seemed at all consistent.

I also don't really see the need to go thru and force-reformat
everything.  Just do it as we run into stuff.  However, I would prefer
it if such reformatting changes were committed separately, and not mixed
in with other real stuff.

[Jacopo Cappellato-4]

On Jun 2, 2012, at 8:10 PM, Adam Heath wrote:

> In the commit you commented on,

Oh no, it was Jacques not me :-)

Jacopo

[Adam Heath-2]

On 06/02/2012 01:14 PM, Jacopo Cappellato wrote:
>
> On Jun 2, 2012, at 8:10 PM, Adam Heath wrote:
>
>> In the commit you commented on,
>
> Oh no, it was Jacques not me :-)
>
> Jacopo

No, really, I don't get you two confused.  I just wasn't paying
attention.  I've only had half a cup of coffee today.

[Jacques Le Roux]

Adam Heath wrote:

> On 06/02/2012 01:06 PM, Jacopo Cappellato wrote:
>>
>> On Jun 2, 2012, at 7:15 PM, Adam Heath wrote:
>>
>>> There is no way that we can ever enforce a single, unified width. Screen sizes change, font sizes change.  It's too political
>>> to come up with a solution. So, the only fallback is to never split.  Then it is up to each end-developer to modify their
>>> display to handle the long lines.
>>
>> I would instead prefer the following guideline: "There is no upper limit to line width; only split long lines if it increases
>> readability of the code".
>
> That's fine too.  I do the same, when embedding a toMap-type call in the
> middle of a method invocation, and I forsee the need to add/remove
> fields in the future.

> In the commit you commented on, I believe I only combined lines that
> seemed to be arbitrarily split on individual method arguments, due to
> some random screen width.  It never really seemed at all consistent.

You could still have cut them at the right place ;o)

> I also don't really see the need to go thru and force-reformat
> everything.  Just do it as we run into stuff.  However, I would prefer
> it if such reformatting changes were committed separately, and not mixed
> in with other real stuff.

Yes I also think it's a bad practise to reformat all: more useless reviews. Only when we rewrite/refactor makes sense or when it's
really bad (seems that it was your case)

OK, let the creativity flows then...

This said Ctrl+I in Eclipse is still your friend. It's not just about max line length, but OK we already follow the Sun convention
mostly

Chapter closed for me

Jacques

[Jacques Le Roux]

Jacques Le Roux wrote:

> Adam Heath wrote:
>> On 06/02/2012 01:06 PM, Jacopo Cappellato wrote:
>>>
>>> On Jun 2, 2012, at 7:15 PM, Adam Heath wrote:
>>>
>>>> There is no way that we can ever enforce a single, unified width. Screen sizes change, font sizes change.  It's too political
>>>> to come up with a solution. So, the only fallback is to never split.  Then it is up to each end-developer to modify their
>>>> display to handle the long lines.
>>>
>>> I would instead prefer the following guideline: "There is no upper limit to line width; only split long lines if it increases
>>> readability of the code".
>>
>> That's fine too.  I do the same, when embedding a toMap-type call in the
>> middle of a method invocation, and I forsee the need to add/remove
>> fields in the future.
>
>> In the commit you commented on, I believe I only combined lines that
>> seemed to be arbitrarily split on individual method arguments, due to
>> some random screen width.  It never really seemed at all consistent.
>
> You could still have cut them at the right place ;o)
>
>> I also don't really see the need to go thru and force-reformat
>> everything.  Just do it as we run into stuff.  However, I would prefer
>> it if such reformatting changes were committed separately, and not mixed
>> in with other real stuff.
>
> Yes I also think it's a bad practise to reformat all: more useless reviews. Only when we rewrite/refactor makes sense or when it's
> really bad (seems that it was your case)
>
> OK, let the creativity flows then...
>
> This said Ctrl+I in Eclipse is still your friend. It's not just about max line length, but OK we already follow the Sun convention
> mostly

I meant Ctrl+Shift+F (default formatter key), since Ctrl+I  is only about indentation.

Jacques

> Chapter closed for me
>
> Jacques