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" <[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() {
