OK, done finally, needed a cast
return "true".equalsIgnoreCase((String)AIMProperties.get("testReq"));
When there are no conventions, it's always a trade off between readability
(using more verbose forms) and conciness. Sometime
conciness is better, it's the case here
BTW, I found this advice
http://docs.sun.com/app/docs/doc/819-3681/abebf?a=view#abebm is there any
performance advantages (I reckon
it's would anyway be marginal) ?
Thanks for helping this out and sorry to have bothered some
Jacques
De : "Jacques Le Roux" <[EMAIL PROTECTED]>
> Yes, true. I saw it when Jonathon send his message and then forgot. So it
> will simpley be
>
> return "true".equalsIgnoreCase(AIMProperties.get("testReq"));
>
> Finally Scoot was right : a bit miserable :o) Though, I was not aware of
> equalsIgnoreCase... Lesson learned...
>
> Jacques
>
>
> De : "Jacopo Cappellato" <[EMAIL PROTECTED]>
> > Jacques Le Roux wrote:
> > > De : "Jacopo Cappellato" <[EMAIL PROTECTED]>
> > >> Jonathon -- Improov wrote:
> > >>> Why not this?
> > >> +1
> > >
> > > I will dot it soon using my preferred one (mix of Jonathon's and Adrian's)
> > > return testReq == null ? false :
> > > "true".equalsIgnoreCase(AIMProperties.get("testReq"));
> >
> > You can do the same with:
> >
> > "true".equalsIgnoreCase(AIMProperties.get("testReq"));
> >
> > in fact, if AIMProperties.get("testReq") is null then the
> > equalsIgnoreCase method will return false.
> >
> > Jacopo
> >
> >
> > >
> > >>> That would work no matter what case, upper or lower or mixed. Unless we
> > >>> don't want case to be ignored?
> > >> Yes, this is one doubt I have: who is setting the "testReq" parameter?
> > >> Why it was initially compared to TRUE and not true? Is there a reason or
> > >> just a bug?
> > >
> > > I will look at that too
> > >
> > > Jacques
> > >
> > >> Jacopo
> > >>
> > >>
> > >>> The above is also better than:
> > >>>
> > >>> testReq.equalsIgnoreCase("true");
> > >>>
> > >>> The 1st statement doesn't require any testing of "testReq" for null
> > >>> value. The 2nd statement will bomb if "testReq" is null.
> > >>>
> > >>> I think I'm getting more and more lost in this thread. Time to bug out.
> > >>> :)
> > >>>
> > >>> Jonathon
> > >>>
> > >>> BJ Freeman wrote:
> > >>>> first, the orgninal code would never evaluate since lowercase true is
> > >>>> correct.
> > >>>> return ("TRUE".equals((String) should be return ("true".equals((String)
> > >>>> second if the properties is null it would not evaluate correct, and
> > >>>> there is not use using more cpu cycles to evaluate.
> > >>>> if(testReq.equals("TRUE")) this is operator error thought I had changed
> > >>>> it like I did in versiion 4.0
> > >>>> if(testReq.toUpperCase().equals("TRUE"))
> > >>>> proably should have been
> > >>>> if(testReq.tolowerCase().equals("true"))
> > >>>> the tolowerCase make sure if a user puts in TRUE is will still get
> > >>>> evaluated properly.
> > >>>>
> > >>>>
> > >>>> Jacopo Cappellato sent the following on 12/19/2007 2:09 AM:
> > >>>>> Jacques, BJ,
> > >>>>>
> > >>>>> after having read the comments in the issue and the commit logs I
> > >>>>> really
> > >>>>> don't understand what was the bug and how this patch is going to fix
> > >>>>> it.
> > >>>>> Please, see my comments below:
> > >>>>>
> > >>>>> Jacques Le Roux wrote:
> > >>>>>> David,
> > >>>>>>
> > >>>>>> 1. I had already refactored the code, please see trunk rev. 605190
> > >>>>>> and
> > >>>>>> release4.0 rev. 605189. BTW there are tons and tons of such
> > >>>>>> bad code formating eveywhere in the code...
> > >>>>> This is an exaggeration and by the way this is not a good reason for
> > >>>>> adding new ones
> > >>>>>
> > >>>>>> 2. I let BJ answer, personally I would put false but I did not know
> > >>>>>> why BJ put this so I let it.
> > >>>>> This is alone a good reason to not commit in the trunk and release
> > >>>>> branch.
> > >>>>>
> > >>>>>> 3. I even could have rewritten it
> > >>>>>> "TRUE".equals(testReq.toUpperCase()) ? true : false;
> > >>>>>> but I did not thought it was such important
> > >>>>>>
> > >>>>> After a very quick look, in my opinion, the best code snippet was the
> > >>>>> one modified by the patch.
> > >>>>>
> > >>>>> Jacopo
> > >>>>>
> > >>>>>> Jacques
> > >>>>>>
> > >>>>>>
> > >>>>>> De : "David E Jones" <[EMAIL PROTECTED]>
> > >>>>>>> 1. Bad code formating
> > >>>>>>> 2. Makes the default true, is that what we really want?
> > >>>>>>> 3. If 2 is true then should use more compact and easy to read,
> > >>>>>>> like if != false instead of if = true
> > >>>>>>>
> > >>>>>>> -David
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> On Tue, 18 Dec 2007 11:37:55 -0000
> > >>>>>>> [EMAIL PROTECTED] wrote:
> > >>>>>>>
> > >>>>>>>> Author: jleroux
> > >>>>>>>> Date: Tue Dec 18 03:37:47 2007
> > >>>>>>>> New Revision: 605186
> > >>>>>>>>
> > >>>>>>>> URL: http://svn.apache.org/viewvc?rev=605186&view=rev
> > >>>>>>>> Log:
> > >>>>>>>> A patch from BJ Freeman "Allows better testing of testmode from
> > >>>>>>>> propties file of
> > >>>>>>>> authorize.net" (https://issues.apache.org/jira/browse/OFBIZ-1450) -
> > >>>>>>>> OFBIZ-1450
> > >>>>>>>>
> > >>>>>>>> Modified:
> > >>>>>>>>
> > >>>>>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> Modified:
> > >>>>>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> URL:
> > >>>>>>>>
> > >
>
http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java?rev=605186&r1=605185&r2=605186&view=diff
> > >>>>>>
> > >>>>>>>> ==============================================================================
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> ---
> > >>>>>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> (original) +++
> > >>>>>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> Tue Dec 18 03:37:47 2007 @@ -376,7 +376,15 @@ } private static
> > >>>>>>>> boolean isTestMode() {
> > >>>>>>>> - return ("TRUE".equals((String)
> > >>>>>>>> AIMProperties.get("testReq")));
> > >>>>>>>> + boolean ret = true;
> > >>>>>>>> + String testReq = (String)AIMProperties.get("testReq");
> > >>>>>>>> + if(testReq != null) {
> > >>>>>>>> + if(testReq.equals("TRUE"))
> > >>>>>>>> + ret = true;
> > >>>>>>>> + else
> > >>>>>>>> + ret = false;
> > >>>>>>>> + }
> > >>>>>>>> + return ret;
> > >>>>>>>> }
> > >>>>>>>>
> > >>>>>>>> private static String getVersion() {
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>
> >
>