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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>:
>
> > 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
> >
> >
> >
>
