You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Scott Gray <le...@gmail.com> on 2008/06/03 08:18:42 UTC
Re: svn commit: r662654 - in /ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit: showvisits.groovy visitdetail.groovy
Hi Ashish
Everything looks fine, just a couple of (hopefully) helpful tips:
There is a shortened version of java ternary operator available "?:" which
provides a default if the rhs of an assignment resolves to null or false:
Instead of
showAll = parameters.showAll;
if (!showAll) {
showAll = "false";
}
You can use
showAll = parameters.showAll ?: "false";
context.showAll = showAll;
With regards to things like this:
viewSize = Integer.valueOf((String) parameters.VIEW_SIZE).intValue();
I think, but I haven't tried it yet, that we can do this instead:
viewSize = parameters.VIEW_SIZE as int;
Instead of
visitList = new ArrayList();
we can do
visitList = []; // and for maps it's [:]
but I've been meaning to ask on the dev list if all Maps and Lists should be
instances of FastMap and FastList for better performance or whether it's not
worth it for scripts
Regards
Scott
2008/6/3 <as...@apache.org>:
> Author: ashish
> Date: Mon Jun 2 22:19:27 2008
> New Revision: 662654
>
> URL: http://svn.apache.org/viewvc?rev=662654&view=rev
> Log:
> Here comes the First conversion from my side on *.bsh to *.groovy. (Party
> of JIRA issue # OFBIZ-1801)
> Contents are from Party component.
>
> PS :- Scott can you please review some of my commits in the initial phase ?
> Its my first chance to work in Groovy.
> I will try to improve myself in groovy ASAP.
>
> Thanks in advance.
>
> Modified:
>
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
>
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
>
> Modified:
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy?rev=662654&r1=662653&r2=662654&view=diff
>
> ==============================================================================
> ---
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
> (original)
> +++
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
> Mon Jun 2 22:19:27 2008
> @@ -17,33 +17,31 @@
> * under the License.
> */
>
> -import org.ofbiz.base.util.*;
> -import org.ofbiz.entity.*;
> -import org.ofbiz.entity.util.*;
> +import org.ofbiz.entity.util.EntityFindOptions;
> import org.ofbiz.entity.condition.*;
> -import org.ofbiz.entity.transaction.*;
> +import org.ofbiz.entity.transaction.TransactionUtil;
>
> module = "showvisits.bsh";
>
> -partyId = parameters.get("partyId");
> -context.put("partyId", partyId);
> +partyId = parameters.partyId;
> +context.partyId = partyId;
>
> -showAll = parameters.get("showAll");
> -if (showAll == null) showAll = "false";
> -context.put("showAll", showAll);
> +showAll = parameters.showAll;
> +if (!showAll) showAll = "false";
> +context.showAll = showAll;
>
> -sort = parameters.get("sort");
> -context.put("sort", sort);
> +sort = parameters.sort;
> +context.sort = sort;
>
> visitListIt = null;
> -sortList = UtilMisc.toList("-fromDate");
> -if (sort != null) sortList.add(0, sort);
> +sortList = ["-fromDate"];
> +if (sort) sortList.add(0, sort);
>
> boolean beganTransaction = false;
> try {
> beganTransaction = TransactionUtil.begin();
>
> - if (partyId != null) {
> + if (partyId) {
> visitListIt = delegator.find("Visit",
> EntityCondition.makeCondition("partyId", EntityOperator.EQUALS, partyId),
> null, null, sortList, new EntityFindOptions(true,
> EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
> EntityFindOptions.CONCUR_READ_ONLY, true));
> } else if (showAll.equalsIgnoreCase("true")) {
> visitListIt = delegator.find("Visit", null, null, null, sortList,
> new EntityFindOptions(true, EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
> EntityFindOptions.CONCUR_READ_ONLY, true));
> @@ -55,18 +53,18 @@
> viewIndex = 1;
> viewSize = 20;
> try {
> - viewIndex = Integer.valueOf((String)
> parameters.get("VIEW_INDEX")).intValue();
> + viewIndex = Integer.valueOf((String)
> parameters.VIEW_INDEX).intValue();
> } catch (Exception e) {
> viewIndex = 1;
> }
> - context.put("viewIndex", viewIndex);
> + context.viewIndex = viewIndex;
>
> try {
> - viewSize = Integer.valueOf((String)
> parameters.get("VIEW_SIZE")).intValue();
> + viewSize = Integer.valueOf((String)
> parameters.VIEW_SIZE).intValue();
> } catch (Exception e) {
> viewSize = 20;
> }
> - context.put("viewSize", viewSize);
> + context.viewSize = viewSize;
>
> // get the indexes for the partial list
> lowIndex = (((viewIndex - 1) * viewSize) + 1);
> @@ -74,7 +72,7 @@
>
> // get the partial list for this page
> visitList = visitListIt.getPartialList(lowIndex, viewSize);
> - if (visitList == null) {
> + if (!visitList) {
> visitList = new ArrayList();
> }
>
> @@ -84,16 +82,16 @@
> if (highIndex > visitListSize) {
> highIndex = visitListSize;
> }
> - context.put("visitSize", visitListSize);
> + context.visitSize = visitListSize;
>
> visitListIt.close();
> -} catch (GenericEntityException e) {
> +} catch (Exception e) {
> String errMsg = "Failure in operation, rolling back transaction";
> Debug.logError(e, errMsg, module);
> try {
> // only rollback the transaction if we started one...
> TransactionUtil.rollback(beganTransaction, errMsg, e);
> - } catch (GenericEntityException e2) {
> + } catch (Exception e2) {
> Debug.logError(e2, "Could not rollback transaction: " +
> e2.toString(), module);
> }
> // after rolling back, rethrow the exception
> @@ -103,14 +101,14 @@
> TransactionUtil.commit(beganTransaction);
> }
>
> -context.put("visitList", visitList);
> -if (visitList != null) {
> +context.visitList = visitList;
> +if (visitList) {
> listSize = lowIndex + visitList.size();
> }
>
> if (listSize < highIndex) {
> highIndex = listSize;
> }
> -context.put("lowIndex", lowIndex);
> -context.put("highIndex", highIndex);
> -context.put("listSize", listSize);
> +context.lowIndex = lowIndex;
> +context.highIndex = highIndex;
> +context.listSize = listSize;
>
> Modified:
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy?rev=662654&r1=662653&r2=662654&view=diff
>
> ==============================================================================
> ---
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
> (original)
> +++
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
> Mon Jun 2 22:19:27 2008
> @@ -17,39 +17,34 @@
> * under the License.
> */
>
> -import org.ofbiz.base.util.*;
> -import org.ofbiz.entity.*;
> -import org.ofbiz.entity.util.*;
> -import org.ofbiz.entity.condition.*;
> -
> -partyId = parameters.get("partyId");
> -visitId = parameters.get("visitId");
> +partyId = parameters.partyId;
> +visitId = parameters.visitId;
>
> visit = null;
> serverHits = null;
> -if (visitId != null) {
> - visit = delegator.findByPrimaryKey("Visit", UtilMisc.toMap("visitId",
> visitId));
> - if (visit != null) {
> - serverHits = delegator.findByAnd("ServerHit",
> UtilMisc.toMap("visitId", visitId), UtilMisc.toList("-hitStartDateTime"));
> +if (visitId) {
> + visit = delegator.findByPrimaryKey("Visit", [visitId : visitId]);
> + if (visit) {
> + serverHits = delegator.findByAnd("ServerHit", [visitId : visitId],
> ["-hitStartDateTime"]);
> }
> }
>
> viewIndex = 0;
> try {
> - viewIndex = Integer.valueOf((String)
> parameters.get("VIEW_INDEX")).intValue();
> + viewIndex = Integer.valueOf((String)
> parameters.VIEW_INDEX).intValue();
> } catch (Exception e) {
> viewIndex = 0;
> }
>
> viewSize = 20;
> try {
> - viewSize = Integer.valueOf((String)
> parameters.get("VIEW_SIZE")).intValue();
> + viewSize = Integer.valueOf((String) parameters.VIEW_SIZE).intValue();
> } catch (Exception e) {
> viewSize = 20;
> }
>
> listSize = 0;
> -if (serverHits != null) {
> +if (serverHits) {
> listSize = serverHits.size();
> }
> lowIndex = viewIndex * viewSize;
> @@ -58,13 +53,13 @@
> highIndex = listSize;
> }
>
> -context.put("partyId", partyId);
> -context.put("visitId", visitId);
> -context.put("visit", visit);
> -context.put("serverHits", serverHits);
> -
> -context.put("viewIndex", viewIndex);
> -context.put("viewSize", viewSize);
> -context.put("listSize", listSize);
> -context.put("lowIndex", lowIndex);
> -context.put("highIndex", highIndex);
> +context.partyId = partyId;
> +context.visitId = visitId;
> +context.visit = visit;
> +context.serverHits = serverHits;
> +
> +context.viewIndex = viewIndex;
> +context.viewSize = viewSize;
> +context.listSize = listSize;
> +context.lowIndex = lowIndex;
> +context.highIndex = highIndex;
> \ No newline at end of file
>
>
>
Re: svn commit: r662654 - in /ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit: showvisits.groovy visitdetail.groovy
Posted by Jacques Le Roux <ja...@les7arts.com>.
+1
Jacques
From: "Joe Eckard" <jo...@redrocketcorp.com>
> Since we're still in the initial stage of converting beanshell scripts to groovy, I would like to propose that when renaming
> files, we uppercase the filenames. (e.g product.bsh becomes Product.groovy)
>
> The main reason for this is that when a script is run without a class declaration, the filename is used to create the class name
> and you can experience problems with untyped variables. For example, a script named product.groovy becomes class "product", and
> if the script contains an untyped variable "product", it assumes you're trying to access the class "product" instead of a new
> variable "product".
>
> A secondary reason would be consistency, as some scripts have already been named this way. (EditProductFeatures.groovy, etc.)
>
>
> -Joe
>
> On Jun 3, 2008, at 2:37 AM, Ashish Vijaywargiya wrote:
>
>> Scott,
>>
>> I am feeling very happy to see your positive comments on my commit.
>> I noted down the points specified by you and will try to include those in my
>> upcoming commit in this area.
>>
>> --
>> Ashish
>>
>> On Tue, Jun 3, 2008 at 2:18 AM, Scott Gray <le...@gmail.com> wrote:
>>
>>> Hi Ashish
>>>
>>> Everything looks fine, just a couple of (hopefully) helpful tips:
>>> There is a shortened version of java ternary operator available "?:" which
>>> provides a default if the rhs of an assignment resolves to null or false:
>>> Instead of
>>> showAll = parameters.showAll;
>>> if (!showAll) {
>>> showAll = "false";
>>> }
>>> You can use
>>> showAll = parameters.showAll ?: "false";
>>> context.showAll = showAll;
>>>
>>> With regards to things like this:
>>> viewSize = Integer.valueOf((String) parameters.VIEW_SIZE).intValue();
>>> I think, but I haven't tried it yet, that we can do this instead:
>>> viewSize = parameters.VIEW_SIZE as int;
>>>
>>> Instead of
>>> visitList = new ArrayList();
>>> we can do
>>> visitList = []; // and for maps it's [:]
>>> but I've been meaning to ask on the dev list if all Maps and Lists should
>>> be
>>> instances of FastMap and FastList for better performance or whether it's
>>> not
>>> worth it for scripts
>>>
>>> Regards
>>> Scott
>>>
>>> 2008/6/3 <as...@apache.org>:
>>>
>>>> Author: ashish
>>>> Date: Mon Jun 2 22:19:27 2008
>>>> New Revision: 662654
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=662654&view=rev
>>>> Log:
>>>> Here comes the First conversion from my side on *.bsh to *.groovy. (Party
>>>> of JIRA issue # OFBIZ-1801)
>>>> Contents are from Party component.
>>>>
>>>> PS :- Scott can you please review some of my commits in the initial phase
>>> ?
>>>> Its my first chance to work in Groovy.
>>>> I will try to improve myself in groovy ASAP.
>>>>
>>>> Thanks in advance.
>>>>
>>>> Modified:
>>>>
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/showvisits.groovy
>>>>
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/visitdetail.groovy
>>>>
>>>> Modified:
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/showvisits.groovy
>>>> URL:
>>>>
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy?rev=662654&r1=662653&r2=662654&view=diff
>>>>
>>>>
>>> = = = = = = = = = =====================================================================
>>>> ---
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/showvisits.groovy
>>>> (original)
>>>> +++
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/showvisits.groovy
>>>> Mon Jun 2 22:19:27 2008
>>>> @@ -17,33 +17,31 @@
>>>> * under the License.
>>>> */
>>>>
>>>> -import org.ofbiz.base.util.*;
>>>> -import org.ofbiz.entity.*;
>>>> -import org.ofbiz.entity.util.*;
>>>> +import org.ofbiz.entity.util.EntityFindOptions;
>>>> import org.ofbiz.entity.condition.*;
>>>> -import org.ofbiz.entity.transaction.*;
>>>> +import org.ofbiz.entity.transaction.TransactionUtil;
>>>>
>>>> module = "showvisits.bsh";
>>>>
>>>> -partyId = parameters.get("partyId");
>>>> -context.put("partyId", partyId);
>>>> +partyId = parameters.partyId;
>>>> +context.partyId = partyId;
>>>>
>>>> -showAll = parameters.get("showAll");
>>>> -if (showAll == null) showAll = "false";
>>>> -context.put("showAll", showAll);
>>>> +showAll = parameters.showAll;
>>>> +if (!showAll) showAll = "false";
>>>> +context.showAll = showAll;
>>>>
>>>> -sort = parameters.get("sort");
>>>> -context.put("sort", sort);
>>>> +sort = parameters.sort;
>>>> +context.sort = sort;
>>>>
>>>> visitListIt = null;
>>>> -sortList = UtilMisc.toList("-fromDate");
>>>> -if (sort != null) sortList.add(0, sort);
>>>> +sortList = ["-fromDate"];
>>>> +if (sort) sortList.add(0, sort);
>>>>
>>>> boolean beganTransaction = false;
>>>> try {
>>>> beganTransaction = TransactionUtil.begin();
>>>>
>>>> - if (partyId != null) {
>>>> + if (partyId) {
>>>> visitListIt = delegator.find("Visit",
>>>> EntityCondition.makeCondition("partyId", EntityOperator.EQUALS, partyId),
>>>> null, null, sortList, new EntityFindOptions(true,
>>>> EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
>>>> EntityFindOptions.CONCUR_READ_ONLY, true));
>>>> } else if (showAll.equalsIgnoreCase("true")) {
>>>> visitListIt = delegator.find("Visit", null, null, null, sortList,
>>>> new EntityFindOptions(true, EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
>>>> EntityFindOptions.CONCUR_READ_ONLY, true));
>>>> @@ -55,18 +53,18 @@
>>>> viewIndex = 1;
>>>> viewSize = 20;
>>>> try {
>>>> - viewIndex = Integer.valueOf((String)
>>>> parameters.get("VIEW_INDEX")).intValue();
>>>> + viewIndex = Integer.valueOf((String)
>>>> parameters.VIEW_INDEX).intValue();
>>>> } catch (Exception e) {
>>>> viewIndex = 1;
>>>> }
>>>> - context.put("viewIndex", viewIndex);
>>>> + context.viewIndex = viewIndex;
>>>>
>>>> try {
>>>> - viewSize = Integer.valueOf((String)
>>>> parameters.get("VIEW_SIZE")).intValue();
>>>> + viewSize = Integer.valueOf((String)
>>>> parameters.VIEW_SIZE).intValue();
>>>> } catch (Exception e) {
>>>> viewSize = 20;
>>>> }
>>>> - context.put("viewSize", viewSize);
>>>> + context.viewSize = viewSize;
>>>>
>>>> // get the indexes for the partial list
>>>> lowIndex = (((viewIndex - 1) * viewSize) + 1);
>>>> @@ -74,7 +72,7 @@
>>>>
>>>> // get the partial list for this page
>>>> visitList = visitListIt.getPartialList(lowIndex, viewSize);
>>>> - if (visitList == null) {
>>>> + if (!visitList) {
>>>> visitList = new ArrayList();
>>>> }
>>>>
>>>> @@ -84,16 +82,16 @@
>>>> if (highIndex > visitListSize) {
>>>> highIndex = visitListSize;
>>>> }
>>>> - context.put("visitSize", visitListSize);
>>>> + context.visitSize = visitListSize;
>>>>
>>>> visitListIt.close();
>>>> -} catch (GenericEntityException e) {
>>>> +} catch (Exception e) {
>>>> String errMsg = "Failure in operation, rolling back transaction";
>>>> Debug.logError(e, errMsg, module);
>>>> try {
>>>> // only rollback the transaction if we started one...
>>>> TransactionUtil.rollback(beganTransaction, errMsg, e);
>>>> - } catch (GenericEntityException e2) {
>>>> + } catch (Exception e2) {
>>>> Debug.logError(e2, "Could not rollback transaction: " +
>>>> e2.toString(), module);
>>>> }
>>>> // after rolling back, rethrow the exception
>>>> @@ -103,14 +101,14 @@
>>>> TransactionUtil.commit(beganTransaction);
>>>> }
>>>>
>>>> -context.put("visitList", visitList);
>>>> -if (visitList != null) {
>>>> +context.visitList = visitList;
>>>> +if (visitList) {
>>>> listSize = lowIndex + visitList.size();
>>>> }
>>>>
>>>> if (listSize < highIndex) {
>>>> highIndex = listSize;
>>>> }
>>>> -context.put("lowIndex", lowIndex);
>>>> -context.put("highIndex", highIndex);
>>>> -context.put("listSize", listSize);
>>>> +context.lowIndex = lowIndex;
>>>> +context.highIndex = highIndex;
>>>> +context.listSize = listSize;
>>>>
>>>> Modified:
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/visitdetail.groovy
>>>> URL:
>>>>
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy?rev=662654&r1=662653&r2=662654&view=diff
>>>>
>>>>
>>> = = = = = = = = = =====================================================================
>>>> ---
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/visitdetail.groovy
>>>> (original)
>>>> +++
>>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/ visit/visitdetail.groovy
>>>> Mon Jun 2 22:19:27 2008
>>>> @@ -17,39 +17,34 @@
>>>> * under the License.
>>>> */
>>>>
>>>> -import org.ofbiz.base.util.*;
>>>> -import org.ofbiz.entity.*;
>>>> -import org.ofbiz.entity.util.*;
>>>> -import org.ofbiz.entity.condition.*;
>>>> -
>>>> -partyId = parameters.get("partyId");
>>>> -visitId = parameters.get("visitId");
>>>> +partyId = parameters.partyId;
>>>> +visitId = parameters.visitId;
>>>>
>>>> visit = null;
>>>> serverHits = null;
>>>> -if (visitId != null) {
>>>> - visit = delegator.findByPrimaryKey("Visit",
>>> UtilMisc.toMap("visitId",
>>>> visitId));
>>>> - if (visit != null) {
>>>> - serverHits = delegator.findByAnd("ServerHit",
>>>> UtilMisc.toMap("visitId", visitId),
>>> UtilMisc.toList("-hitStartDateTime"));
>>>> +if (visitId) {
>>>> + visit = delegator.findByPrimaryKey("Visit", [visitId : visitId]);
>>>> + if (visit) {
>>>> + serverHits = delegator.findByAnd("ServerHit", [visitId :
>>> visitId],
>>>> ["-hitStartDateTime"]);
>>>> }
>>>> }
>>>>
>>>> viewIndex = 0;
>>>> try {
>>>> - viewIndex = Integer.valueOf((String)
>>>> parameters.get("VIEW_INDEX")).intValue();
>>>> + viewIndex = Integer.valueOf((String)
>>>> parameters.VIEW_INDEX).intValue();
>>>> } catch (Exception e) {
>>>> viewIndex = 0;
>>>> }
>>>>
>>>> viewSize = 20;
>>>> try {
>>>> - viewSize = Integer.valueOf((String)
>>>> parameters.get("VIEW_SIZE")).intValue();
>>>> + viewSize = Integer.valueOf((String)
>>> parameters.VIEW_SIZE).intValue();
>>>> } catch (Exception e) {
>>>> viewSize = 20;
>>>> }
>>>>
>>>> listSize = 0;
>>>> -if (serverHits != null) {
>>>> +if (serverHits) {
>>>> listSize = serverHits.size();
>>>> }
>>>> lowIndex = viewIndex * viewSize;
>>>> @@ -58,13 +53,13 @@
>>>> highIndex = listSize;
>>>> }
>>>>
>>>> -context.put("partyId", partyId);
>>>> -context.put("visitId", visitId);
>>>> -context.put("visit", visit);
>>>> -context.put("serverHits", serverHits);
>>>> -
>>>> -context.put("viewIndex", viewIndex);
>>>> -context.put("viewSize", viewSize);
>>>> -context.put("listSize", listSize);
>>>> -context.put("lowIndex", lowIndex);
>>>> -context.put("highIndex", highIndex);
>>>> +context.partyId = partyId;
>>>> +context.visitId = visitId;
>>>> +context.visit = visit;
>>>> +context.serverHits = serverHits;
>>>> +
>>>> +context.viewIndex = viewIndex;
>>>> +context.viewSize = viewSize;
>>>> +context.listSize = listSize;
>>>> +context.lowIndex = lowIndex;
>>>> +context.highIndex = highIndex;
>>>> \ No newline at end of file
>>>>
>>>>
>>>>
>>>
>
>
Re: svn commit: r662654 - in /ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit: showvisits.groovy visitdetail.groovy
Posted by Joe Eckard <jo...@redrocketcorp.com>.
Since we're still in the initial stage of converting beanshell scripts
to groovy, I would like to propose that when renaming files, we
uppercase the filenames. (e.g product.bsh becomes Product.groovy)
The main reason for this is that when a script is run without a class
declaration, the filename is used to create the class name and you can
experience problems with untyped variables. For example, a script
named product.groovy becomes class "product", and if the script
contains an untyped variable "product", it assumes you're trying to
access the class "product" instead of a new variable "product".
A secondary reason would be consistency, as some scripts have already
been named this way. (EditProductFeatures.groovy, etc.)
-Joe
On Jun 3, 2008, at 2:37 AM, Ashish Vijaywargiya wrote:
> Scott,
>
> I am feeling very happy to see your positive comments on my commit.
> I noted down the points specified by you and will try to include
> those in my
> upcoming commit in this area.
>
> --
> Ashish
>
> On Tue, Jun 3, 2008 at 2:18 AM, Scott Gray <le...@gmail.com> wrote:
>
>> Hi Ashish
>>
>> Everything looks fine, just a couple of (hopefully) helpful tips:
>> There is a shortened version of java ternary operator available
>> "?:" which
>> provides a default if the rhs of an assignment resolves to null or
>> false:
>> Instead of
>> showAll = parameters.showAll;
>> if (!showAll) {
>> showAll = "false";
>> }
>> You can use
>> showAll = parameters.showAll ?: "false";
>> context.showAll = showAll;
>>
>> With regards to things like this:
>> viewSize = Integer.valueOf((String) parameters.VIEW_SIZE).intValue();
>> I think, but I haven't tried it yet, that we can do this instead:
>> viewSize = parameters.VIEW_SIZE as int;
>>
>> Instead of
>> visitList = new ArrayList();
>> we can do
>> visitList = []; // and for maps it's [:]
>> but I've been meaning to ask on the dev list if all Maps and Lists
>> should
>> be
>> instances of FastMap and FastList for better performance or whether
>> it's
>> not
>> worth it for scripts
>>
>> Regards
>> Scott
>>
>> 2008/6/3 <as...@apache.org>:
>>
>>> Author: ashish
>>> Date: Mon Jun 2 22:19:27 2008
>>> New Revision: 662654
>>>
>>> URL: http://svn.apache.org/viewvc?rev=662654&view=rev
>>> Log:
>>> Here comes the First conversion from my side on *.bsh to *.groovy.
>>> (Party
>>> of JIRA issue # OFBIZ-1801)
>>> Contents are from Party component.
>>>
>>> PS :- Scott can you please review some of my commits in the
>>> initial phase
>> ?
>>> Its my first chance to work in Groovy.
>>> I will try to improve myself in groovy ASAP.
>>>
>>> Thanks in advance.
>>>
>>> Modified:
>>>
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/showvisits.groovy
>>>
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/visitdetail.groovy
>>>
>>> Modified:
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/showvisits.groovy
>>> URL:
>>>
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy?rev=662654&r1=662653&r2=662654&view=diff
>>>
>>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>>> ---
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/showvisits.groovy
>>> (original)
>>> +++
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/showvisits.groovy
>>> Mon Jun 2 22:19:27 2008
>>> @@ -17,33 +17,31 @@
>>> * under the License.
>>> */
>>>
>>> -import org.ofbiz.base.util.*;
>>> -import org.ofbiz.entity.*;
>>> -import org.ofbiz.entity.util.*;
>>> +import org.ofbiz.entity.util.EntityFindOptions;
>>> import org.ofbiz.entity.condition.*;
>>> -import org.ofbiz.entity.transaction.*;
>>> +import org.ofbiz.entity.transaction.TransactionUtil;
>>>
>>> module = "showvisits.bsh";
>>>
>>> -partyId = parameters.get("partyId");
>>> -context.put("partyId", partyId);
>>> +partyId = parameters.partyId;
>>> +context.partyId = partyId;
>>>
>>> -showAll = parameters.get("showAll");
>>> -if (showAll == null) showAll = "false";
>>> -context.put("showAll", showAll);
>>> +showAll = parameters.showAll;
>>> +if (!showAll) showAll = "false";
>>> +context.showAll = showAll;
>>>
>>> -sort = parameters.get("sort");
>>> -context.put("sort", sort);
>>> +sort = parameters.sort;
>>> +context.sort = sort;
>>>
>>> visitListIt = null;
>>> -sortList = UtilMisc.toList("-fromDate");
>>> -if (sort != null) sortList.add(0, sort);
>>> +sortList = ["-fromDate"];
>>> +if (sort) sortList.add(0, sort);
>>>
>>> boolean beganTransaction = false;
>>> try {
>>> beganTransaction = TransactionUtil.begin();
>>>
>>> - if (partyId != null) {
>>> + if (partyId) {
>>> visitListIt = delegator.find("Visit",
>>> EntityCondition.makeCondition("partyId", EntityOperator.EQUALS,
>>> partyId),
>>> null, null, sortList, new EntityFindOptions(true,
>>> EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
>>> EntityFindOptions.CONCUR_READ_ONLY, true));
>>> } else if (showAll.equalsIgnoreCase("true")) {
>>> visitListIt = delegator.find("Visit", null, null, null,
>>> sortList,
>>> new EntityFindOptions(true,
>>> EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
>>> EntityFindOptions.CONCUR_READ_ONLY, true));
>>> @@ -55,18 +53,18 @@
>>> viewIndex = 1;
>>> viewSize = 20;
>>> try {
>>> - viewIndex = Integer.valueOf((String)
>>> parameters.get("VIEW_INDEX")).intValue();
>>> + viewIndex = Integer.valueOf((String)
>>> parameters.VIEW_INDEX).intValue();
>>> } catch (Exception e) {
>>> viewIndex = 1;
>>> }
>>> - context.put("viewIndex", viewIndex);
>>> + context.viewIndex = viewIndex;
>>>
>>> try {
>>> - viewSize = Integer.valueOf((String)
>>> parameters.get("VIEW_SIZE")).intValue();
>>> + viewSize = Integer.valueOf((String)
>>> parameters.VIEW_SIZE).intValue();
>>> } catch (Exception e) {
>>> viewSize = 20;
>>> }
>>> - context.put("viewSize", viewSize);
>>> + context.viewSize = viewSize;
>>>
>>> // get the indexes for the partial list
>>> lowIndex = (((viewIndex - 1) * viewSize) + 1);
>>> @@ -74,7 +72,7 @@
>>>
>>> // get the partial list for this page
>>> visitList = visitListIt.getPartialList(lowIndex, viewSize);
>>> - if (visitList == null) {
>>> + if (!visitList) {
>>> visitList = new ArrayList();
>>> }
>>>
>>> @@ -84,16 +82,16 @@
>>> if (highIndex > visitListSize) {
>>> highIndex = visitListSize;
>>> }
>>> - context.put("visitSize", visitListSize);
>>> + context.visitSize = visitListSize;
>>>
>>> visitListIt.close();
>>> -} catch (GenericEntityException e) {
>>> +} catch (Exception e) {
>>> String errMsg = "Failure in operation, rolling back transaction";
>>> Debug.logError(e, errMsg, module);
>>> try {
>>> // only rollback the transaction if we started one...
>>> TransactionUtil.rollback(beganTransaction, errMsg, e);
>>> - } catch (GenericEntityException e2) {
>>> + } catch (Exception e2) {
>>> Debug.logError(e2, "Could not rollback transaction: " +
>>> e2.toString(), module);
>>> }
>>> // after rolling back, rethrow the exception
>>> @@ -103,14 +101,14 @@
>>> TransactionUtil.commit(beganTransaction);
>>> }
>>>
>>> -context.put("visitList", visitList);
>>> -if (visitList != null) {
>>> +context.visitList = visitList;
>>> +if (visitList) {
>>> listSize = lowIndex + visitList.size();
>>> }
>>>
>>> if (listSize < highIndex) {
>>> highIndex = listSize;
>>> }
>>> -context.put("lowIndex", lowIndex);
>>> -context.put("highIndex", highIndex);
>>> -context.put("listSize", listSize);
>>> +context.lowIndex = lowIndex;
>>> +context.highIndex = highIndex;
>>> +context.listSize = listSize;
>>>
>>> Modified:
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/visitdetail.groovy
>>> URL:
>>>
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy?rev=662654&r1=662653&r2=662654&view=diff
>>>
>>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>>> ---
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/visitdetail.groovy
>>> (original)
>>> +++
>>>
>> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/
>> visit/visitdetail.groovy
>>> Mon Jun 2 22:19:27 2008
>>> @@ -17,39 +17,34 @@
>>> * under the License.
>>> */
>>>
>>> -import org.ofbiz.base.util.*;
>>> -import org.ofbiz.entity.*;
>>> -import org.ofbiz.entity.util.*;
>>> -import org.ofbiz.entity.condition.*;
>>> -
>>> -partyId = parameters.get("partyId");
>>> -visitId = parameters.get("visitId");
>>> +partyId = parameters.partyId;
>>> +visitId = parameters.visitId;
>>>
>>> visit = null;
>>> serverHits = null;
>>> -if (visitId != null) {
>>> - visit = delegator.findByPrimaryKey("Visit",
>> UtilMisc.toMap("visitId",
>>> visitId));
>>> - if (visit != null) {
>>> - serverHits = delegator.findByAnd("ServerHit",
>>> UtilMisc.toMap("visitId", visitId),
>> UtilMisc.toList("-hitStartDateTime"));
>>> +if (visitId) {
>>> + visit = delegator.findByPrimaryKey("Visit", [visitId :
>>> visitId]);
>>> + if (visit) {
>>> + serverHits = delegator.findByAnd("ServerHit", [visitId :
>> visitId],
>>> ["-hitStartDateTime"]);
>>> }
>>> }
>>>
>>> viewIndex = 0;
>>> try {
>>> - viewIndex = Integer.valueOf((String)
>>> parameters.get("VIEW_INDEX")).intValue();
>>> + viewIndex = Integer.valueOf((String)
>>> parameters.VIEW_INDEX).intValue();
>>> } catch (Exception e) {
>>> viewIndex = 0;
>>> }
>>>
>>> viewSize = 20;
>>> try {
>>> - viewSize = Integer.valueOf((String)
>>> parameters.get("VIEW_SIZE")).intValue();
>>> + viewSize = Integer.valueOf((String)
>> parameters.VIEW_SIZE).intValue();
>>> } catch (Exception e) {
>>> viewSize = 20;
>>> }
>>>
>>> listSize = 0;
>>> -if (serverHits != null) {
>>> +if (serverHits) {
>>> listSize = serverHits.size();
>>> }
>>> lowIndex = viewIndex * viewSize;
>>> @@ -58,13 +53,13 @@
>>> highIndex = listSize;
>>> }
>>>
>>> -context.put("partyId", partyId);
>>> -context.put("visitId", visitId);
>>> -context.put("visit", visit);
>>> -context.put("serverHits", serverHits);
>>> -
>>> -context.put("viewIndex", viewIndex);
>>> -context.put("viewSize", viewSize);
>>> -context.put("listSize", listSize);
>>> -context.put("lowIndex", lowIndex);
>>> -context.put("highIndex", highIndex);
>>> +context.partyId = partyId;
>>> +context.visitId = visitId;
>>> +context.visit = visit;
>>> +context.serverHits = serverHits;
>>> +
>>> +context.viewIndex = viewIndex;
>>> +context.viewSize = viewSize;
>>> +context.listSize = listSize;
>>> +context.lowIndex = lowIndex;
>>> +context.highIndex = highIndex;
>>> \ No newline at end of file
>>>
>>>
>>>
>>
Re: svn commit: r662654 - in /ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit: showvisits.groovy visitdetail.groovy
Posted by Ashish Vijaywargiya <vi...@gmail.com>.
Scott,
I am feeling very happy to see your positive comments on my commit.
I noted down the points specified by you and will try to include those in my
upcoming commit in this area.
--
Ashish
On Tue, Jun 3, 2008 at 2:18 AM, Scott Gray <le...@gmail.com> wrote:
> Hi Ashish
>
> Everything looks fine, just a couple of (hopefully) helpful tips:
> There is a shortened version of java ternary operator available "?:" which
> provides a default if the rhs of an assignment resolves to null or false:
> Instead of
> showAll = parameters.showAll;
> if (!showAll) {
> showAll = "false";
> }
> You can use
> showAll = parameters.showAll ?: "false";
> context.showAll = showAll;
>
> With regards to things like this:
> viewSize = Integer.valueOf((String) parameters.VIEW_SIZE).intValue();
> I think, but I haven't tried it yet, that we can do this instead:
> viewSize = parameters.VIEW_SIZE as int;
>
> Instead of
> visitList = new ArrayList();
> we can do
> visitList = []; // and for maps it's [:]
> but I've been meaning to ask on the dev list if all Maps and Lists should
> be
> instances of FastMap and FastList for better performance or whether it's
> not
> worth it for scripts
>
> Regards
> Scott
>
> 2008/6/3 <as...@apache.org>:
>
> > Author: ashish
> > Date: Mon Jun 2 22:19:27 2008
> > New Revision: 662654
> >
> > URL: http://svn.apache.org/viewvc?rev=662654&view=rev
> > Log:
> > Here comes the First conversion from my side on *.bsh to *.groovy. (Party
> > of JIRA issue # OFBIZ-1801)
> > Contents are from Party component.
> >
> > PS :- Scott can you please review some of my commits in the initial phase
> ?
> > Its my first chance to work in Groovy.
> > I will try to improve myself in groovy ASAP.
> >
> > Thanks in advance.
> >
> > Modified:
> >
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
> >
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
> >
> > Modified:
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
> > URL:
> >
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy?rev=662654&r1=662653&r2=662654&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
> > (original)
> > +++
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/showvisits.groovy
> > Mon Jun 2 22:19:27 2008
> > @@ -17,33 +17,31 @@
> > * under the License.
> > */
> >
> > -import org.ofbiz.base.util.*;
> > -import org.ofbiz.entity.*;
> > -import org.ofbiz.entity.util.*;
> > +import org.ofbiz.entity.util.EntityFindOptions;
> > import org.ofbiz.entity.condition.*;
> > -import org.ofbiz.entity.transaction.*;
> > +import org.ofbiz.entity.transaction.TransactionUtil;
> >
> > module = "showvisits.bsh";
> >
> > -partyId = parameters.get("partyId");
> > -context.put("partyId", partyId);
> > +partyId = parameters.partyId;
> > +context.partyId = partyId;
> >
> > -showAll = parameters.get("showAll");
> > -if (showAll == null) showAll = "false";
> > -context.put("showAll", showAll);
> > +showAll = parameters.showAll;
> > +if (!showAll) showAll = "false";
> > +context.showAll = showAll;
> >
> > -sort = parameters.get("sort");
> > -context.put("sort", sort);
> > +sort = parameters.sort;
> > +context.sort = sort;
> >
> > visitListIt = null;
> > -sortList = UtilMisc.toList("-fromDate");
> > -if (sort != null) sortList.add(0, sort);
> > +sortList = ["-fromDate"];
> > +if (sort) sortList.add(0, sort);
> >
> > boolean beganTransaction = false;
> > try {
> > beganTransaction = TransactionUtil.begin();
> >
> > - if (partyId != null) {
> > + if (partyId) {
> > visitListIt = delegator.find("Visit",
> > EntityCondition.makeCondition("partyId", EntityOperator.EQUALS, partyId),
> > null, null, sortList, new EntityFindOptions(true,
> > EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
> > EntityFindOptions.CONCUR_READ_ONLY, true));
> > } else if (showAll.equalsIgnoreCase("true")) {
> > visitListIt = delegator.find("Visit", null, null, null, sortList,
> > new EntityFindOptions(true, EntityFindOptions.TYPE_SCROLL_INSENSITIVE,
> > EntityFindOptions.CONCUR_READ_ONLY, true));
> > @@ -55,18 +53,18 @@
> > viewIndex = 1;
> > viewSize = 20;
> > try {
> > - viewIndex = Integer.valueOf((String)
> > parameters.get("VIEW_INDEX")).intValue();
> > + viewIndex = Integer.valueOf((String)
> > parameters.VIEW_INDEX).intValue();
> > } catch (Exception e) {
> > viewIndex = 1;
> > }
> > - context.put("viewIndex", viewIndex);
> > + context.viewIndex = viewIndex;
> >
> > try {
> > - viewSize = Integer.valueOf((String)
> > parameters.get("VIEW_SIZE")).intValue();
> > + viewSize = Integer.valueOf((String)
> > parameters.VIEW_SIZE).intValue();
> > } catch (Exception e) {
> > viewSize = 20;
> > }
> > - context.put("viewSize", viewSize);
> > + context.viewSize = viewSize;
> >
> > // get the indexes for the partial list
> > lowIndex = (((viewIndex - 1) * viewSize) + 1);
> > @@ -74,7 +72,7 @@
> >
> > // get the partial list for this page
> > visitList = visitListIt.getPartialList(lowIndex, viewSize);
> > - if (visitList == null) {
> > + if (!visitList) {
> > visitList = new ArrayList();
> > }
> >
> > @@ -84,16 +82,16 @@
> > if (highIndex > visitListSize) {
> > highIndex = visitListSize;
> > }
> > - context.put("visitSize", visitListSize);
> > + context.visitSize = visitListSize;
> >
> > visitListIt.close();
> > -} catch (GenericEntityException e) {
> > +} catch (Exception e) {
> > String errMsg = "Failure in operation, rolling back transaction";
> > Debug.logError(e, errMsg, module);
> > try {
> > // only rollback the transaction if we started one...
> > TransactionUtil.rollback(beganTransaction, errMsg, e);
> > - } catch (GenericEntityException e2) {
> > + } catch (Exception e2) {
> > Debug.logError(e2, "Could not rollback transaction: " +
> > e2.toString(), module);
> > }
> > // after rolling back, rethrow the exception
> > @@ -103,14 +101,14 @@
> > TransactionUtil.commit(beganTransaction);
> > }
> >
> > -context.put("visitList", visitList);
> > -if (visitList != null) {
> > +context.visitList = visitList;
> > +if (visitList) {
> > listSize = lowIndex + visitList.size();
> > }
> >
> > if (listSize < highIndex) {
> > highIndex = listSize;
> > }
> > -context.put("lowIndex", lowIndex);
> > -context.put("highIndex", highIndex);
> > -context.put("listSize", listSize);
> > +context.lowIndex = lowIndex;
> > +context.highIndex = highIndex;
> > +context.listSize = listSize;
> >
> > Modified:
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
> > URL:
> >
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy?rev=662654&r1=662653&r2=662654&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
> > (original)
> > +++
> >
> ofbiz/trunk/applications/party/webapp/partymgr/WEB-INF/actions/visit/visitdetail.groovy
> > Mon Jun 2 22:19:27 2008
> > @@ -17,39 +17,34 @@
> > * under the License.
> > */
> >
> > -import org.ofbiz.base.util.*;
> > -import org.ofbiz.entity.*;
> > -import org.ofbiz.entity.util.*;
> > -import org.ofbiz.entity.condition.*;
> > -
> > -partyId = parameters.get("partyId");
> > -visitId = parameters.get("visitId");
> > +partyId = parameters.partyId;
> > +visitId = parameters.visitId;
> >
> > visit = null;
> > serverHits = null;
> > -if (visitId != null) {
> > - visit = delegator.findByPrimaryKey("Visit",
> UtilMisc.toMap("visitId",
> > visitId));
> > - if (visit != null) {
> > - serverHits = delegator.findByAnd("ServerHit",
> > UtilMisc.toMap("visitId", visitId),
> UtilMisc.toList("-hitStartDateTime"));
> > +if (visitId) {
> > + visit = delegator.findByPrimaryKey("Visit", [visitId : visitId]);
> > + if (visit) {
> > + serverHits = delegator.findByAnd("ServerHit", [visitId :
> visitId],
> > ["-hitStartDateTime"]);
> > }
> > }
> >
> > viewIndex = 0;
> > try {
> > - viewIndex = Integer.valueOf((String)
> > parameters.get("VIEW_INDEX")).intValue();
> > + viewIndex = Integer.valueOf((String)
> > parameters.VIEW_INDEX).intValue();
> > } catch (Exception e) {
> > viewIndex = 0;
> > }
> >
> > viewSize = 20;
> > try {
> > - viewSize = Integer.valueOf((String)
> > parameters.get("VIEW_SIZE")).intValue();
> > + viewSize = Integer.valueOf((String)
> parameters.VIEW_SIZE).intValue();
> > } catch (Exception e) {
> > viewSize = 20;
> > }
> >
> > listSize = 0;
> > -if (serverHits != null) {
> > +if (serverHits) {
> > listSize = serverHits.size();
> > }
> > lowIndex = viewIndex * viewSize;
> > @@ -58,13 +53,13 @@
> > highIndex = listSize;
> > }
> >
> > -context.put("partyId", partyId);
> > -context.put("visitId", visitId);
> > -context.put("visit", visit);
> > -context.put("serverHits", serverHits);
> > -
> > -context.put("viewIndex", viewIndex);
> > -context.put("viewSize", viewSize);
> > -context.put("listSize", listSize);
> > -context.put("lowIndex", lowIndex);
> > -context.put("highIndex", highIndex);
> > +context.partyId = partyId;
> > +context.visitId = visitId;
> > +context.visit = visit;
> > +context.serverHits = serverHits;
> > +
> > +context.viewIndex = viewIndex;
> > +context.viewSize = viewSize;
> > +context.listSize = listSize;
> > +context.lowIndex = lowIndex;
> > +context.highIndex = highIndex;
> > \ No newline at end of file
> >
> >
> >
>