You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2007/12/18 12:37:55 UTC
svn commit: r605186 -
/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
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() {
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by BJ Freeman <bj...@free-man.net>.
Jacopo Cappellato sent the following on 12/19/2007 3:18 AM:
> 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)
>
> ok
>
>> second if the properties is null it would not evaluate correct, and
>> there is not use using more cpu cycles to evaluate.
>
> What do you mean with "would not evaluate correct"?
> The original code, if the passed in parameter is null, would have
> returned false.
that was my ignorance on java ternary handling nulls.. guess need to dig
into code to see how it handles things like that.
>
> Jacopo
>
>> 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" <jo...@undersunconsulting.com>
>>>>> 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
>>>>> jleroux@apache.org 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() {
>>>>>>
>>>>>>
>>>
>>>
>>>
>>>
>
>
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacopo Cappellato <ti...@sastau.it>.
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)
ok
> second if the properties is null it would not evaluate correct, and
> there is not use using more cpu cycles to evaluate.
What do you mean with "would not evaluate correct"?
The original code, if the passed in parameter is null, would have
returned false.
Jacopo
> 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" <jo...@undersunconsulting.com>
>>>> 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
>>>> jleroux@apache.org 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() {
>>>>>
>>>>>
>>
>>
>>
>>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
I find it quite intersting :p
Jacques
De : "Scott Gray" <le...@gmail.com>
> Perfect, could someone commit it and put this poor thread out of it's misery
> :-)
>
> Scott
>
> On 20/12/2007, Jonathon -- Improov <jo...@improov.com> wrote:
> >
> > Why not this?
> >
> > "true".equalsIgnoreCase(AIMProperties.get("testReq"));
> >
> > That would work no matter what case, upper or lower or mixed. Unless we
> > don't want case to be ignored?
> >
> >
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Scott Gray <le...@gmail.com>.
Perfect, could someone commit it and put this poor thread out of it's misery
:-)
Scott
On 20/12/2007, Jonathon -- Improov <jo...@improov.com> wrote:
>
> Why not this?
>
> "true".equalsIgnoreCase(AIMProperties.get("testReq"));
>
> That would work no matter what case, upper or lower or mixed. Unless we
> don't want case to be ignored?
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
> 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) ?
Ok, forget it : http://www-128.ibm.com/developerworks/java/library/j-jtp1029.html
Jacques
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
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" <ja...@les7arts.com>
> 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" <ti...@sastau.it>
> > Jacques Le Roux wrote:
> > > De : "Jacopo Cappellato" <ti...@sastau.it>
> > >> 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" <jo...@undersunconsulting.com>
> > >>>>>>> 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
> > >>>>>>> jleroux@apache.org 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() {
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>
> >
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
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" <ti...@sastau.it>
> Jacques Le Roux wrote:
> > De : "Jacopo Cappellato" <ti...@sastau.it>
> >> 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" <jo...@undersunconsulting.com>
> >>>>>>> 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
> >>>>>>> jleroux@apache.org 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() {
> >>>>>>>>
> >>>>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacopo Cappellato <ti...@sastau.it>.
Jacques Le Roux wrote:
> De : "Jacopo Cappellato" <ti...@sastau.it>
>> 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" <jo...@undersunconsulting.com>
>>>>>>> 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
>>>>>>> jleroux@apache.org 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() {
>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
De : "Jacopo Cappellato" <ti...@sastau.it>
> 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"));
> > 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" <jo...@undersunconsulting.com>
> >>>>> 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
> >>>>> jleroux@apache.org 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() {
> >>>>>>
> >>>>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacopo Cappellato <ti...@sastau.it>.
Jonathon -- Improov wrote:
> Why not this?
>
> "true".equalsIgnoreCase(AIMProperties.get("testReq"));
+1
>
> 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?
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" <jo...@undersunconsulting.com>
>>>>> 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
>>>>> jleroux@apache.org 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() {
>>>>>>
>>>>>>
>>>
>>>
>>>
>>>
>>
>>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jonathon -- Improov <jo...@improov.com>.
Why not this?
"true".equalsIgnoreCase(AIMProperties.get("testReq"));
That would work no matter what case, upper or lower or mixed. Unless we don't want case to be ignored?
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" <jo...@undersunconsulting.com>
>>>> 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
>>>> jleroux@apache.org 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() {
>>>>>
>>>>>
>>
>>
>>
>>
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by BJ Freeman <bj...@free-man.net>.
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" <jo...@undersunconsulting.com>
>>> 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
>>> jleroux@apache.org 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() {
>>>>
>>>>
>
>
>
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Scott Gray <le...@gmail.com>.
Big +1 here, we should never commit anything that we don't completely
understand, it should almost be as if we had written the code ourselves. A
committer is answerable for his commits not the contributor.
Regards
Scott
On 19/12/2007, Jacopo Cappellato <ti...@sastau.it> wrote:
>
> Jacques Le Roux wrote:
> > 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.
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by BJ Freeman <bj...@free-man.net>.
The main motivation was that this would never evaluate to test mode,
which is import when first bringin this code online.
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" <jo...@undersunconsulting.com>
>>> 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
>>> jleroux@apache.org 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() {
>>>>
>>>>
>
>
>
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacopo Cappellato <ti...@sastau.it>.
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" <jo...@undersunconsulting.com>
>> 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
>> jleroux@apache.org 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() {
>>>
>>>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
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...
2. I let BJ answer, personally I would put false but I did not know why BJ put this so I let it.
3. I even could have rewritten it
"TRUE".equals(testReq.toUpperCase()) ? true : false;
but I did not thought it was such important
Jacques
De : "David E Jones" <jo...@undersunconsulting.com>
>
> 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
> jleroux@apache.org 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() {
> >
> >
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes good, no worry with null in last expression evaluation ? I'm still far from a Java expert :(
Jacques
Ooops I re-read it should have been
return testReq == null ? true : ("TRUE".equals(testReq.toUpperCase()) ? true : false; // (not else at end of course, but I guess you
got it, was obvious)
But I prefer yours anyway !
De : "Adrian Crum" <ad...@hlmksw.com>=
> It seems to me it should be:
>
> return testReq == null ? false : "TRUE".equals(testReq.toUpperCase());
>
> Anything that is not "TRUE" is false.
>
> -Adrian
>
> Jacques Le Roux wrote:
>
> > I thought a bit about that, and since I spoke about APL, that's how it would be written in an APL style in Java :
> >
> > return testReq == null ? true : ("TRUE".equals(testReq.toUpperCase()) ? true : else;
> >
> > (Note I choose to put true if null but maybe it should be false as BJ did...)
> >
> > We used to call this type of style "one liner syndrom". The goal (play?) is to put the as less as characters as possible, it's a
bit
> > better read when written
> >
> > return (testReq == null) ? true : ("TRUE".equals(testReq.toUpperCase()) ? true : else;
> >
> > But with Java you can't really have fun with this, it's not APL...
> >
> > Maybe some may argue that to use blocks and brackets still makes sense since it's far more readable (at a glance)...
> > It was always a dilemma for me when I was writting APL and C or Pascal in the same day... I did not take the straight road,
it's
> > not the one I prefer...
> >
> > Jacques
> >
> > PS : sorry for bothering :p
> >
> >
> >>Yes clearer than my proposition using ternary operator and no worry about default value : + 1
> >>We can play a lot with thing like this, I wrote thousands of APL lines code, I'm used to play with these kind of boolean
stuffes,
> >
> > I
> >
> >>must say that from this POV, APL is more fun than any other languages ;o)
> >>
> >>Jacques
> >>
> >>----- Message d'origine -----
> >>De : "Adrian Crum" <ad...@hlmksw.com>
> >>À : <de...@ofbiz.apache.org>
> >>Envoyé : mardi 18 décembre 2007 19:26
> >>Objet : Re: svn commit: r605186 -
> >>/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> >>
> >>
> >>
> >>>Plus, you can eliminate one if() construct:
> >>>
> >>>if (testReq != null) {
> >>> return "TRUE".equals(testReq.toUpperCase());
> >>>}
> >>>
> >>>-Adrian
> >>>
> >>>David E Jones wrote:
> >>>
> >>>
> >>>>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
> >>>>jleroux@apache.org 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/AIMPaymentSe
r
> >>
> >>vices.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() {
> >>>>>
> >>>>>
> >>>>
> >>>>
> >
> >
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Adrian Crum <ad...@hlmksw.com>.
It seems to me it should be:
return testReq == null ? false : "TRUE".equals(testReq.toUpperCase());
Anything that is not "TRUE" is false.
-Adrian
Jacques Le Roux wrote:
> I thought a bit about that, and since I spoke about APL, that's how it would be written in an APL style in Java :
>
> return testReq == null ? true : ("TRUE".equals(testReq.toUpperCase()) ? true : else;
>
> (Note I choose to put true if null but maybe it should be false as BJ did...)
>
> We used to call this type of style "one liner syndrom". The goal (play?) is to put the as less as characters as possible, it's a bit
> better read when written
>
> return (testReq == null) ? true : ("TRUE".equals(testReq.toUpperCase()) ? true : else;
>
> But with Java you can't really have fun with this, it's not APL...
>
> Maybe some may argue that to use blocks and brackets still makes sense since it's far more readable (at a glance)...
> It was always a dilemma for me when I was writting APL and C or Pascal in the same day... I did not take the straight road, it's
> not the one I prefer...
>
> Jacques
>
> PS : sorry for bothering :p
>
>
>>Yes clearer than my proposition using ternary operator and no worry about default value : + 1
>>We can play a lot with thing like this, I wrote thousands of APL lines code, I'm used to play with these kind of boolean stuffes,
>
> I
>
>>must say that from this POV, APL is more fun than any other languages ;o)
>>
>>Jacques
>>
>>----- Message d'origine -----
>>De : "Adrian Crum" <ad...@hlmksw.com>
>>À : <de...@ofbiz.apache.org>
>>Envoyé : mardi 18 décembre 2007 19:26
>>Objet : Re: svn commit: r605186 -
>>/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>
>>
>>
>>>Plus, you can eliminate one if() construct:
>>>
>>>if (testReq != null) {
>>> return "TRUE".equals(testReq.toUpperCase());
>>>}
>>>
>>>-Adrian
>>>
>>>David E Jones wrote:
>>>
>>>
>>>>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
>>>>jleroux@apache.org 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/AIMPaymentSer
>>
>>vices.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() {
>>>>>
>>>>>
>>>>
>>>>
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
I thought a bit about that, and since I spoke about APL, that's how it would be written in an APL style in Java :
return testReq == null ? true : ("TRUE".equals(testReq.toUpperCase()) ? true : else;
(Note I choose to put true if null but maybe it should be false as BJ did...)
We used to call this type of style "one liner syndrom". The goal (play?) is to put the as less as characters as possible, it's a bit
better read when written
return (testReq == null) ? true : ("TRUE".equals(testReq.toUpperCase()) ? true : else;
But with Java you can't really have fun with this, it's not APL...
Maybe some may argue that to use blocks and brackets still makes sense since it's far more readable (at a glance)...
It was always a dilemma for me when I was writting APL and C or Pascal in the same day... I did not take the straight road, it's
not the one I prefer...
Jacques
PS : sorry for bothering :p
> Yes clearer than my proposition using ternary operator and no worry about default value : + 1
> We can play a lot with thing like this, I wrote thousands of APL lines code, I'm used to play with these kind of boolean stuffes,
I
> must say that from this POV, APL is more fun than any other languages ;o)
>
> Jacques
>
> ----- Message d'origine -----
> De : "Adrian Crum" <ad...@hlmksw.com>
> À : <de...@ofbiz.apache.org>
> Envoyé : mardi 18 décembre 2007 19:26
> Objet : Re: svn commit: r605186 -
> /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>
>
> > Plus, you can eliminate one if() construct:
> >
> > if (testReq != null) {
> > return "TRUE".equals(testReq.toUpperCase());
> > }
> >
> > -Adrian
> >
> > David E Jones wrote:
> >
> > > 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
> > > jleroux@apache.org 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/AIMPaymentSer
> vices.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() {
> > >>
> > >>
> > >
> > >
> >
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes clearer than my proposition using ternary operator and no worry about default value : + 1
We can play a lot with thing like this, I wrote thousands of APL lines code, I'm used to play with these kind of boolean stuffes, I
must say that from this POV, APL is more fun than any other languages ;o)
Jacques
----- Message d'origine -----
De : "Adrian Crum" <ad...@hlmksw.com>
À : <de...@ofbiz.apache.org>
Envoyé : mardi 18 décembre 2007 19:26
Objet : Re: svn commit: r605186 -
/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> Plus, you can eliminate one if() construct:
>
> if (testReq != null) {
> return "TRUE".equals(testReq.toUpperCase());
> }
>
> -Adrian
>
> David E Jones wrote:
>
> > 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
> > jleroux@apache.org 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/AIMPaymentSer
vices.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() {
> >>
> >>
> >
> >
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Adrian Crum <ad...@hlmksw.com>.
Plus, you can eliminate one if() construct:
if (testReq != null) {
return "TRUE".equals(testReq.toUpperCase());
}
-Adrian
David E Jones wrote:
> 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
> jleroux@apache.org 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() {
>>
>>
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jonathon -- Improov <jo...@improov.com>.
Why was the 1-liner code removed? The original was concise, and was able to take null values
without blowing up.
Or if we're worried about class cast exceptions:
return "TRUE".equals(AIMProperties.get("testReq"));
Jonathon
David E Jones wrote:
> 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
> jleroux@apache.org 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() {
>>
>>
>
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Not both, just one had, I have already dealed with that in trunk rev. 605190 and release4.0 rev. 605189
Jacques
De : "BJ Freeman" <bj...@free-man.net>
> that does not look like my patch.
> mine has an uppcase in it.
>
>
> David E Jones sent the following on 12/18/2007 10:19 AM:
> > 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
> > jleroux@apache.org 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() {
> >>
> >>
> >
> >
> >
>
Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by BJ Freeman <bj...@free-man.net>.
that does not look like my patch.
mine has an uppcase in it.
David E Jones sent the following on 12/18/2007 10:19 AM:
> 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
> jleroux@apache.org 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() {
>>
>>
>
>
>
Re: svn commit: r605186 -
/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by David E Jones <jo...@undersunconsulting.com>.
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
jleroux@apache.org 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() {
>
>
Re: svn commit: r605186 -
/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by David E Jones <jo...@undersunconsulting.com>.
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
jleroux@apache.org 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() {
>
>