Re: svn commit: r1761181 [1/2] - in /ofbiz/trunk/applications/product/src/main/java/org/apache/ofbiz: product/category/ product/category/ftl/ product/config/ product/feature/ product/imagemanagement/ product/inventory/ product/price/ product/product/ produ...

[Jacques Le Roux]

Hi Ashish,

This should be improved, exception in services should use Debug.logError,  use return ServiceUtil.returnError and not swallow the exception


Le 17/09/2016 à 12:34, [hidden email] a écrit :

>               product = EntityQuery.use(delegator).from("Product").where("productId", productId).queryOne();
>           } catch (GenericEntityException e) {
> -            // TODO Auto-generated catch block
>               e.printStackTrace();
>           }
>           if (EntityTypeUtil.hasParentType(delegator, "ProductType", "productTypeId", product.getString("productTypeId"), "parentTypeId", "MARKETING_PKG")) {
>               try {
>                   resultOutput = dispatcher.runSync("getMktgPackagesAvailable", contextInput);
>               } catch (GenericServiceException e) {
> -                // TODO Auto-generated catch block
>                   e.printStackTrace();
>               }
>           } else {
>               try {
>                   resultOutput = dispatcher.runSync("getInventoryAvailableByFacility", contextInput);
>               } catch (GenericServiceException e) {
> -                // TODO Auto-generated catch block
>                   e.printStackTrace();
>               }
>           }
> @@ -910,7 +898,6 @@ public class InventoryServices {
>           try {
>               productPrices = EntityQuery.use(delegator).from("ProductPrice").where("productId",productId).orderBy("-fromDate").cache(true).queryList();
>           } catch (GenericEntityException e) {
> -            // TODO Auto-generated catch block
>               e.printStackTrace();
>           }
>           //change this for product price
> @@ -977,7 +964,6 @@ public class InventoryServices {
>                       EntityOperator.AND);
>                   salesUsageIt = EntityQuery.use(delegator).from(salesUsageViewEntity).where(cond).queryIterator();
>               } catch (GenericEntityException e2) {
> -                // TODO Auto-generated catch block
>                   e2.printStackTrace();
>               }
>  
> @@ -996,7 +982,6 @@ public class InventoryServices {
>               try {
>                   salesUsageIt.close();
>               } catch (GenericEntityException e2) {
> -                // TODO Auto-generated catch block
>                   e2.printStackTrace();
>               }
>  
> @@ -1013,7 +998,6 @@ public class InventoryServices {
>                           EntityOperator.AND);
>                   productionUsageIt = EntityQuery.use(delegator).from(productionUsageViewEntity).where(conditions).queryIterator();
>               } catch (GenericEntityException e1) {
> -                // TODO Auto-generated catch block
>                   e1.printStackTrace();
>               }
>  
> @@ -1032,7 +1016,6 @@ public class InventoryServices {
>               try {
>                   productionUsageIt.close();
>               } catch (GenericEntityException e) {
> -                // TODO Auto-generated catch block
>                   e.printStackTrace();
>               }
>  

Thanks!

Jacques

[Jacques Le Roux]

Hi Ashish,

I think we lose some information here


Le 17/09/2016 à 12:34, [hidden email] a écrit :

>       public static Map<String, Object> prodFindDistinctVariants(DispatchContext dctx, Map<String, ? extends Object> context) {
> -        // * String productId      -- Parent (virtual) product ID
> -        // * String feature        -- Distinct feature name
>           //TODO This service has not yet been implemented.
>           return ServiceUtil.returnFailure();
>       public static Map<String, Object> prodMakeFeatureTree(DispatchContext dctx, Map<String, ? extends Object> context) {
> -        // * String productId      -- Parent (virtual) product ID
> -        // * List featureOrder     -- Order of features
> -        // * Boolean checkInventory-- To calculate available inventory.
> -        // * String productStoreId -- Product Store ID for Inventory
>           String productStoreId = (String) context.get("productStoreId");
>           Locale locale = (Locale) context.get("locale");
>  
> @@ -352,9 +340,6 @@ public class ProductServices {
>        * Gets the product features of a product.
>        */
>       public static Map<String, Object> prodGetFeatures(DispatchContext dctx, Map<String, ? extends Object> context) {
> -        // * String productId      -- Product ID to find
> -        // * String type           -- Type of feature (STANDARD_FEATURE, SELECTABLE_FEATURE)
> -        // * String distinct       -- Distinct feature (SIZE, COLOR)
>           Delegator delegator = dctx.getDelegator();
>           Map<String, Object> result = new HashMap<String, Object>();
>           String productId = (String) context.get("productId");
> @@ -386,7 +371,6 @@ public class ProductServices {
>        * Finds a product by product ID.
>        */
>       public static Map<String, Object> prodFindProduct(DispatchContext dctx, Map<String, ? extends Object> context) {
> -        // * String productId      -- Product ID to find
>           Delegator delegator = dctx.getDelegator();
>           Map<String, Object> result = new HashMap<String, Object>();
>           String productId = (String) context.get("productId");
> @@ -407,15 +391,10 @@ public class ProductServices {

>       public static Map<String, Object> prodFindAssociatedByType(DispatchContext dctx, Map<String, ? extends Object> context) {
> -        // * String productId      -- Current Product ID
> -        // * String type           -- Type of association (ie PRODUCT_UPGRADE, PRODUCT_COMPLEMENT, PRODUCT_VARIANT)

Thanks!

Jacques

[Jacques Le Roux]

Hi Ashish,

The comment above should be removed as well

Le 17/09/2016 à 12:34, [hidden email] a écrit :
>       public String complete(boolean force) throws GeneralException {
>           // clear out empty lines
> -        // this.checkEmptyLines(); // removing, this seems to be causeing issues -  mja
> -

Thanks!

Jacques

[Harsh Vijaywargiya]

Hi Jacques, Ashish

I also noted few classes with such patterns during clean up, and in
first attempt I just cleaned the default TODOs.

I will provide a patch with this and other suggested enhancements.

Thanks & Regards,

Harsh


On Saturday 17 September 2016 04:56 PM, Jacques Le Roux wrote:

> Hi Ashish,
>
> This should be improved, exception in services should use
> Debug.logError,  use return ServiceUtil.returnError and not swallow
> the exception
>
>
> Le 17/09/2016 à 12:34, [hidden email] a écrit :
>>               product =
>> EntityQuery.use(delegator).from("Product").where("productId",
>> productId).queryOne();
>>           } catch (GenericEntityException e) {
>> -            // TODO Auto-generated catch block
>>               e.printStackTrace();
>>           }
>>           if (EntityTypeUtil.hasParentType(delegator, "ProductType",
>> "productTypeId", product.getString("productTypeId"), "parentTypeId",
>> "MARKETING_PKG")) {
>>               try {
>>                   resultOutput =
>> dispatcher.runSync("getMktgPackagesAvailable", contextInput);
>>               } catch (GenericServiceException e) {
>> -                // TODO Auto-generated catch block
>>                   e.printStackTrace();
>>               }
>>           } else {
>>               try {
>>                   resultOutput =
>> dispatcher.runSync("getInventoryAvailableByFacility", contextInput);
>>               } catch (GenericServiceException e) {
>> -                // TODO Auto-generated catch block
>>                   e.printStackTrace();
>>               }
>>           }
>> @@ -910,7 +898,6 @@ public class InventoryServices {
>>           try {
>>               productPrices =
>> EntityQuery.use(delegator).from("ProductPrice").where("productId",productId).orderBy("-fromDate").cache(true).queryList();
>>           } catch (GenericEntityException e) {
>> -            // TODO Auto-generated catch block
>>               e.printStackTrace();
>>           }
>>           //change this for product price
>> @@ -977,7 +964,6 @@ public class InventoryServices {
>>                       EntityOperator.AND);
>>                   salesUsageIt =
>> EntityQuery.use(delegator).from(salesUsageViewEntity).where(cond).queryIterator();
>>               } catch (GenericEntityException e2) {
>> -                // TODO Auto-generated catch block
>>                   e2.printStackTrace();
>>               }
>>   @@ -996,7 +982,6 @@ public class InventoryServices {
>>               try {
>>                   salesUsageIt.close();
>>               } catch (GenericEntityException e2) {
>> -                // TODO Auto-generated catch block
>>                   e2.printStackTrace();
>>               }
>>   @@ -1013,7 +998,6 @@ public class InventoryServices {
>>                           EntityOperator.AND);
>>                   productionUsageIt =
>> EntityQuery.use(delegator).from(productionUsageViewEntity).where(conditions).queryIterator();
>>               } catch (GenericEntityException e1) {
>> -                // TODO Auto-generated catch block
>>                   e1.printStackTrace();
>>               }
>>   @@ -1032,7 +1016,6 @@ public class InventoryServices {
>>               try {
>>                   productionUsageIt.close();
>>               } catch (GenericEntityException e) {
>> -                // TODO Auto-generated catch block
>>                   e.printStackTrace();
>>               }
>
> Thanks!
>
> Jacques
>

[Jacques Le Roux]

Hi Ashish,

At r1763154 I did all the similar trivial changes.

To All, Adam,

I wonder about keeping this code put in in 2008 with http://svn.apache.org/viewvc?view=revision&revision=713573

/**
  * Finds product variants based on a product ID and a distinct feature.
  */
public static Map<String, Object> prodFindDistinctVariants(DispatchContext dctx, Map<String, ? extends Object> context) {
     // * String productId      -- Parent (virtual) product ID
     // * String feature        -- Distinct feature name
     //TODO This service has not yet been implemented.
     return ServiceUtil.returnFailure();
}

Jacques


Le 17/09/2016 à 13:26, Jacques Le Roux a écrit :

> Hi Ashish,
>
> I think we lose some information here
>
>
> Le 17/09/2016 à 12:34, [hidden email] a écrit :
>>       public static Map<String, Object> prodFindDistinctVariants(DispatchContext dctx, Map<String, ? extends Object> context) {
>> -        // * String productId      -- Parent (virtual) product ID
>> -        // * String feature        -- Distinct feature name
>>           //TODO This service has not yet been implemented.
>>           return ServiceUtil.returnFailure();
>>       public static Map<String, Object> prodMakeFeatureTree(DispatchContext dctx, Map<String, ? extends Object> context) {
>> -        // * String productId      -- Parent (virtual) product ID
>> -        // * List featureOrder     -- Order of features
>> -        // * Boolean checkInventory-- To calculate available inventory.
>> -        // * String productStoreId -- Product Store ID for Inventory
>>           String productStoreId = (String) context.get("productStoreId");
>>           Locale locale = (Locale) context.get("locale");
>>   @@ -352,9 +340,6 @@ public class ProductServices {
>>        * Gets the product features of a product.
>>        */
>>       public static Map<String, Object> prodGetFeatures(DispatchContext dctx, Map<String, ? extends Object> context) {
>> -        // * String productId      -- Product ID to find
>> -        // * String type           -- Type of feature (STANDARD_FEATURE, SELECTABLE_FEATURE)
>> -        // * String distinct       -- Distinct feature (SIZE, COLOR)
>>           Delegator delegator = dctx.getDelegator();
>>           Map<String, Object> result = new HashMap<String, Object>();
>>           String productId = (String) context.get("productId");
>> @@ -386,7 +371,6 @@ public class ProductServices {
>>        * Finds a product by product ID.
>>        */
>>       public static Map<String, Object> prodFindProduct(DispatchContext dctx, Map<String, ? extends Object> context) {
>> -        // * String productId      -- Product ID to find
>>           Delegator delegator = dctx.getDelegator();
>>           Map<String, Object> result = new HashMap<String, Object>();
>>           String productId = (String) context.get("productId");
>> @@ -407,15 +391,10 @@ public class ProductServices {
>
>
>>       public static Map<String, Object> prodFindAssociatedByType(DispatchContext dctx, Map<String, ? extends Object> context) {
>> -        // * String productId      -- Current Product ID
>> -        // * String type           -- Type of association (ie PRODUCT_UPGRADE, PRODUCT_COMPLEMENT, PRODUCT_VARIANT)
>
> Thanks!
>
> Jacques
>
>

[Jacques Le Roux]

Hi Harsh,

Any chances about that, should we not create a Jira?

Thanks

Jacques


Le 17/09/2016 à 13:52, Harsh Vijaywargiya a écrit :

> Hi Jacques, Ashish
>
> I also noted few classes with such patterns during clean up, and in first attempt I just cleaned the default TODOs.
>
> I will provide a patch with this and other suggested enhancements.
>
> Thanks & Regards,
>
> Harsh
>
>
> On Saturday 17 September 2016 04:56 PM, Jacques Le Roux wrote:
>> Hi Ashish,
>>
>> This should be improved, exception in services should use Debug.logError,  use return ServiceUtil.returnError and not swallow the exception
>>
>>
>> Le 17/09/2016 à 12:34, [hidden email] a écrit :
>>>               product = EntityQuery.use(delegator).from("Product").where("productId", productId).queryOne();
>>>           } catch (GenericEntityException e) {
>>> -            // TODO Auto-generated catch block
>>>               e.printStackTrace();
>>>           }
>>>           if (EntityTypeUtil.hasParentType(delegator, "ProductType", "productTypeId", product.getString("productTypeId"), "parentTypeId",
>>> "MARKETING_PKG")) {
>>>               try {
>>>                   resultOutput = dispatcher.runSync("getMktgPackagesAvailable", contextInput);
>>>               } catch (GenericServiceException e) {
>>> -                // TODO Auto-generated catch block
>>>                   e.printStackTrace();
>>>               }
>>>           } else {
>>>               try {
>>>                   resultOutput = dispatcher.runSync("getInventoryAvailableByFacility", contextInput);
>>>               } catch (GenericServiceException e) {
>>> -                // TODO Auto-generated catch block
>>>                   e.printStackTrace();
>>>               }
>>>           }
>>> @@ -910,7 +898,6 @@ public class InventoryServices {
>>>           try {
>>>               productPrices =
>>> EntityQuery.use(delegator).from("ProductPrice").where("productId",productId).orderBy("-fromDate").cache(true).queryList();
>>>           } catch (GenericEntityException e) {
>>> -            // TODO Auto-generated catch block
>>>               e.printStackTrace();
>>>           }
>>>           //change this for product price
>>> @@ -977,7 +964,6 @@ public class InventoryServices {
>>>                       EntityOperator.AND);
>>>                   salesUsageIt = EntityQuery.use(delegator).from(salesUsageViewEntity).where(cond).queryIterator();
>>>               } catch (GenericEntityException e2) {
>>> -                // TODO Auto-generated catch block
>>>                   e2.printStackTrace();
>>>               }
>>>   @@ -996,7 +982,6 @@ public class InventoryServices {
>>>               try {
>>>                   salesUsageIt.close();
>>>               } catch (GenericEntityException e2) {
>>> -                // TODO Auto-generated catch block
>>>                   e2.printStackTrace();
>>>               }
>>>   @@ -1013,7 +998,6 @@ public class InventoryServices {
>>>                           EntityOperator.AND);
>>>                   productionUsageIt = EntityQuery.use(delegator).from(productionUsageViewEntity).where(conditions).queryIterator();
>>>               } catch (GenericEntityException e1) {
>>> -                // TODO Auto-generated catch block
>>>                   e1.printStackTrace();
>>>               }
>>>   @@ -1032,7 +1016,6 @@ public class InventoryServices {
>>>               try {
>>>                   productionUsageIt.close();
>>>               } catch (GenericEntityException e) {
>>> -                // TODO Auto-generated catch block
>>>                   e.printStackTrace();
>>>               }
>>
>> Thanks!
>>
>> Jacques
>>
>
>

[Harsh Vijaywargiya]

Hi Jacques,

This is good idea to keep such information on JIRA ticket so anyone can
easily pick it.

I have created https://issues.apache.org/jira/browse/OFBIZ-8341

If I find more such classes then I will add in this ticket, and will
also try to provide the patch for this soon.

Thanks & Regards,

Harsh


On Monday 03 October 2016 04:48 PM, Jacques Le Roux wrote:

> Hi Harsh,
>
> Any chances about that, should we not create a Jira?
>
> Thanks
>
> Jacques
>
>
> Le 17/09/2016 à 13:52, Harsh Vijaywargiya a écrit :
>> Hi Jacques, Ashish
>>
>> I also noted few classes with such patterns during clean up, and in
>> first attempt I just cleaned the default TODOs.
>>
>> I will provide a patch with this and other suggested enhancements.
>>
>> Thanks & Regards,
>>
>> Harsh
>>
>>
>> On Saturday 17 September 2016 04:56 PM, Jacques Le Roux wrote:
>>> Hi Ashish,
>>>
>>> This should be improved, exception in services should use
>>> Debug.logError,  use return ServiceUtil.returnError and not swallow
>>> the exception
>>>
>>>
>>> Le 17/09/2016 à 12:34, [hidden email] a écrit :
>>>>               product =
>>>> EntityQuery.use(delegator).from("Product").where("productId",
>>>> productId).queryOne();
>>>>           } catch (GenericEntityException e) {
>>>> -            // TODO Auto-generated catch block
>>>>               e.printStackTrace();
>>>>           }
>>>>           if (EntityTypeUtil.hasParentType(delegator,
>>>> "ProductType", "productTypeId", product.getString("productTypeId"),
>>>> "parentTypeId", "MARKETING_PKG")) {
>>>>               try {
>>>>                   resultOutput =
>>>> dispatcher.runSync("getMktgPackagesAvailable", contextInput);
>>>>               } catch (GenericServiceException e) {
>>>> -                // TODO Auto-generated catch block
>>>>                   e.printStackTrace();
>>>>               }
>>>>           } else {
>>>>               try {
>>>>                   resultOutput =
>>>> dispatcher.runSync("getInventoryAvailableByFacility", contextInput);
>>>>               } catch (GenericServiceException e) {
>>>> -                // TODO Auto-generated catch block
>>>>                   e.printStackTrace();
>>>>               }
>>>>           }
>>>> @@ -910,7 +898,6 @@ public class InventoryServices {
>>>>           try {
>>>>               productPrices =
>>>> EntityQuery.use(delegator).from("ProductPrice").where("productId",productId).orderBy("-fromDate").cache(true).queryList();
>>>>           } catch (GenericEntityException e) {
>>>> -            // TODO Auto-generated catch block
>>>>               e.printStackTrace();
>>>>           }
>>>>           //change this for product price
>>>> @@ -977,7 +964,6 @@ public class InventoryServices {
>>>>                       EntityOperator.AND);
>>>>                   salesUsageIt =
>>>> EntityQuery.use(delegator).from(salesUsageViewEntity).where(cond).queryIterator();
>>>>               } catch (GenericEntityException e2) {
>>>> -                // TODO Auto-generated catch block
>>>>                   e2.printStackTrace();
>>>>               }
>>>>   @@ -996,7 +982,6 @@ public class InventoryServices {
>>>>               try {
>>>>                   salesUsageIt.close();
>>>>               } catch (GenericEntityException e2) {
>>>> -                // TODO Auto-generated catch block
>>>>                   e2.printStackTrace();
>>>>               }
>>>>   @@ -1013,7 +998,6 @@ public class InventoryServices {
>>>>                           EntityOperator.AND);
>>>>                   productionUsageIt =
>>>> EntityQuery.use(delegator).from(productionUsageViewEntity).where(conditions).queryIterator();
>>>>               } catch (GenericEntityException e1) {
>>>> -                // TODO Auto-generated catch block
>>>>                   e1.printStackTrace();
>>>>               }
>>>>   @@ -1032,7 +1016,6 @@ public class InventoryServices {
>>>>               try {
>>>>                   productionUsageIt.close();
>>>>               } catch (GenericEntityException e) {
>>>> -                // TODO Auto-generated catch block
>>>>                   e.printStackTrace();
>>>>               }
>>>
>>> Thanks!
>>>
>>> Jacques
>>>
>>
>>
>

[Jacques Le Roux]

Since nobody answered, I will remove it in few days as a lazy consensus

Jacques


Le 03/10/2016 à 13:16, Jacques Le Roux a écrit :

> Hi Ashish,
>
> At r1763154 I did all the similar trivial changes.
>
> To All, Adam,
>
> I wonder about keeping this code put in in 2008 with http://svn.apache.org/viewvc?view=revision&revision=713573
>
> /**
>  * Finds product variants based on a product ID and a distinct feature.
>  */
> public static Map<String, Object> prodFindDistinctVariants(DispatchContext dctx, Map<String, ? extends Object> context) {
>     // * String productId      -- Parent (virtual) product ID
>     // * String feature        -- Distinct feature name
>     //TODO This service has not yet been implemented.
>     return ServiceUtil.returnFailure();
> }
>
> Jacques
>
>
> Le 17/09/2016 à 13:26, Jacques Le Roux a écrit :
>> Hi Ashish,
>>
>> I think we lose some information here
>>
>>
>> Le 17/09/2016 à 12:34, [hidden email] a écrit :
>>>       public static Map<String, Object> prodFindDistinctVariants(DispatchContext dctx, Map<String, ? extends Object> context) {
>>> -        // * String productId      -- Parent (virtual) product ID
>>> -        // * String feature        -- Distinct feature name
>>>           //TODO This service has not yet been implemented.
>>>           return ServiceUtil.returnFailure();
>>>       public static Map<String, Object> prodMakeFeatureTree(DispatchContext dctx, Map<String, ? extends Object> context) {
>>> -        // * String productId      -- Parent (virtual) product ID
>>> -        // * List featureOrder     -- Order of features
>>> -        // * Boolean checkInventory-- To calculate available inventory.
>>> -        // * String productStoreId -- Product Store ID for Inventory
>>>           String productStoreId = (String) context.get("productStoreId");
>>>           Locale locale = (Locale) context.get("locale");
>>>   @@ -352,9 +340,6 @@ public class ProductServices {
>>>        * Gets the product features of a product.
>>>        */
>>>       public static Map<String, Object> prodGetFeatures(DispatchContext dctx, Map<String, ? extends Object> context) {
>>> -        // * String productId      -- Product ID to find
>>> -        // * String type           -- Type of feature (STANDARD_FEATURE, SELECTABLE_FEATURE)
>>> -        // * String distinct       -- Distinct feature (SIZE, COLOR)
>>>           Delegator delegator = dctx.getDelegator();
>>>           Map<String, Object> result = new HashMap<String, Object>();
>>>           String productId = (String) context.get("productId");
>>> @@ -386,7 +371,6 @@ public class ProductServices {
>>>        * Finds a product by product ID.
>>>        */
>>>       public static Map<String, Object> prodFindProduct(DispatchContext dctx, Map<String, ? extends Object> context) {
>>> -        // * String productId      -- Product ID to find
>>>           Delegator delegator = dctx.getDelegator();
>>>           Map<String, Object> result = new HashMap<String, Object>();
>>>           String productId = (String) context.get("productId");
>>> @@ -407,15 +391,10 @@ public class ProductServices {
>>
>>
>>>       public static Map<String, Object> prodFindAssociatedByType(DispatchContext dctx, Map<String, ? extends Object> context) {
>>> -        // * String productId      -- Current Product ID
>>> -        // * String type           -- Type of association (ie PRODUCT_UPGRADE, PRODUCT_COMPLEMENT, PRODUCT_VARIANT)
>>
>> Thanks!
>>
>> Jacques
>>
>>
>
>