Re: svn commit: r605186 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java

[Jonathon -- Improov]

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