Guard blocks are bliss.
-Adrian --- On Wed, 8/4/10, [hidden email] <[hidden email]> wrote: > From: [hidden email] <[hidden email]> > Subject: svn commit: r982141 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > To: [hidden email] > Date: Wednesday, August 4, 2010, 12:27 AM > Author: lektran > Date: Wed Aug 4 07:27:49 2010 > New Revision: 982141 > > URL: http://svn.apache.org/viewvc?rev=982141&view=rev > Log: > Replace a long "if not empty" block with an "if empty" > early return, no functional changes > > Modified: > > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > > Modified: > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java?rev=982141&r1=982140&r2=982141&view=diff > ============================================================================== > --- > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > (original) > +++ > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > Wed Aug 4 07:27:49 2010 > @@ -293,60 +293,63 @@ public class PartyWorker { > > List<GenericValue> addresses = > EntityUtil.filterByDate(delegator.findList("PartyAndPostalAddress", > addrCond, null, sort, null, false)); > > //Debug.log("Checking for matching > address: " + addrCond.toString() + "[" + addresses.size() + > "]", module); > > + if > (UtilValidate.isEmpty(addresses)) { > + // No address > matches, return an empty list > + return > returnList; > + } > + > > List<GenericValue> validFound = > FastList.newInstance(); > - if > (UtilValidate.isNotEmpty(addresses)) { > - // check the > address line > - for > (GenericValue address: addresses) { > - // > address 1 field > - > String addr1Source = > PartyWorker.makeMatchingString(delegator, address1); > - > String addr1Target = > PartyWorker.makeMatchingString(delegator, > address.getString("address1")); > - > - if > (addr1Target != null) { > - > Debug.log("Comparing address1 : " + > addr1Source + " / " + addr1Target, module); > - > if (addr1Target.equals(addr1Source)) { > - > - > // address 2 field > - > if (address2 != null) { > - > String addr2Source > = PartyWorker.makeMatchingString(delegator, address2); > - > String addr2Target > = PartyWorker.makeMatchingString(delegator, > address.getString("address2")); > - > if (addr2Target != > null) { > - > > Debug.log("Comparing address2 : " + addr2Source + " / " + > addr2Target, module); > - > - > if > (addr2Source.equals(addr2Target)) { > - > > Debug.log("Matching address2; adding valid > address", module); > - > > validFound.add(address); > - > > //validParty.put(address.getString("partyId"), > address.getString("contactMechId")); > - > } > - > } > - > } else { > - > if > (address.get("address2") == null) { > - > > Debug.log("No address2; adding valid address", module); > + // check the address line > + for (GenericValue address: > addresses) { > + // address 1 > field > + String > addr1Source = PartyWorker.makeMatchingString(delegator, > address1); > + String > addr1Target = PartyWorker.makeMatchingString(delegator, > address.getString("address1")); > + > + if (addr1Target > != null) { > + > Debug.log("Comparing address1 : " + addr1Source + " / " + > addr1Target, module); > + if > (addr1Target.equals(addr1Source)) { > + > + > // address 2 field > + > if (address2 != null) { > + > String addr2Source = > PartyWorker.makeMatchingString(delegator, address2); > + > String addr2Target = > PartyWorker.makeMatchingString(delegator, > address.getString("address2")); > + > if (addr2Target != null) { > + > > Debug.log("Comparing address2 : " + addr2Source + " / " + > addr2Target, module); > + > + > if > (addr2Source.equals(addr2Target)) { > + > > Debug.log("Matching address2; adding valid address", > module); > > > validFound.add(address); > > > //validParty.put(address.getString("partyId"), > address.getString("contactMechId")); > > } > > } > + > } else { > + > if (address.get("address2") == > null) { > + > Debug.log("No > address2; adding valid address", module); > + > > validFound.add(address); > + > > //validParty.put(address.getString("partyId"), > address.getString("contactMechId")); > + > } > > } > > } > } > + } > > - if > (UtilValidate.isNotEmpty(validFound)) { > - > for (GenericValue partyAndAddr: validFound) { > - > String partyId = > partyAndAddr.getString("partyId"); > - > if (UtilValidate.isNotEmpty(partyId)) { > - > GenericValue p = > delegator.findByPrimaryKey("Person", > UtilMisc.toMap("partyId", partyId)); > - > if (p != null) { > - > String fName = > p.getString("firstName"); > - > String lName = > p.getString("lastName"); > - > String mName = > p.getString("middleName"); > - > if > (lName.toUpperCase().equals(lastName.toUpperCase())) { > - > if > (fName.toUpperCase().equals(firstName.toUpperCase())) { > - > > if (mName != null && middleName != > null) { > - > > if > (mName.toUpperCase().equals(middleName.toUpperCase())) { > - > > > returnList.add(partyAndAddr); > - > > } > - > > } else if (middleName == null) { > + if > (UtilValidate.isNotEmpty(validFound)) { > + for > (GenericValue partyAndAddr: validFound) { > + > String partyId = partyAndAddr.getString("partyId"); > + if > (UtilValidate.isNotEmpty(partyId)) { > + > GenericValue p = > delegator.findByPrimaryKey("Person", > UtilMisc.toMap("partyId", partyId)); > + > if (p != null) { > + > String fName = > p.getString("firstName"); > + > String lName = > p.getString("lastName"); > + > String mName = > p.getString("middleName"); > + > if > (lName.toUpperCase().equals(lastName.toUpperCase())) { > + > if > (fName.toUpperCase().equals(firstName.toUpperCase())) { > + > if > (mName != null && middleName != null) { > + > > if > (mName.toUpperCase().equals(middleName.toUpperCase())) { > > > > returnList.add(partyAndAddr); > > > } > + > } > else if (middleName == null) { > + > > returnList.add(partyAndAddr); > > > } > > } > > } > @@ -355,6 +358,7 @@ public class PartyWorker { > } > } > > + > return returnList; > } > > > > |
Long conditionals are certainly no fun :-)
Regards Scott On 4/08/2010, at 7:31 PM, Adrian Crum wrote: > Guard blocks are bliss. > > -Adrian > > --- On Wed, 8/4/10, [hidden email] <[hidden email]> wrote: > >> From: [hidden email] <[hidden email]> >> Subject: svn commit: r982141 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java >> To: [hidden email] >> Date: Wednesday, August 4, 2010, 12:27 AM >> Author: lektran >> Date: Wed Aug 4 07:27:49 2010 >> New Revision: 982141 >> >> URL: http://svn.apache.org/viewvc?rev=982141&view=rev >> Log: >> Replace a long "if not empty" block with an "if empty" >> early return, no functional changes >> >> Modified: >> >> ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java >> >> Modified: >> ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java?rev=982141&r1=982140&r2=982141&view=diff >> ============================================================================== >> --- >> ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java >> (original) >> +++ >> ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java >> Wed Aug 4 07:27:49 2010 >> @@ -293,60 +293,63 @@ public class PartyWorker { >> >> List<GenericValue> addresses = >> EntityUtil.filterByDate(delegator.findList("PartyAndPostalAddress", >> addrCond, null, sort, null, false)); >> >> //Debug.log("Checking for matching >> address: " + addrCond.toString() + "[" + addresses.size() + >> "]", module); >> >> + if >> (UtilValidate.isEmpty(addresses)) { >> + // No address >> matches, return an empty list >> + return >> returnList; >> + } >> + >> >> List<GenericValue> validFound = >> FastList.newInstance(); >> - if >> (UtilValidate.isNotEmpty(addresses)) { >> - // check the >> address line >> - for >> (GenericValue address: addresses) { >> - // >> address 1 field >> - >> String addr1Source = >> PartyWorker.makeMatchingString(delegator, address1); >> - >> String addr1Target = >> PartyWorker.makeMatchingString(delegator, >> address.getString("address1")); >> - >> - if >> (addr1Target != null) { >> - >> Debug.log("Comparing address1 : " + >> addr1Source + " / " + addr1Target, module); >> - >> if (addr1Target.equals(addr1Source)) { >> - >> - >> // address 2 field >> - >> if (address2 != null) { >> - >> String addr2Source >> = PartyWorker.makeMatchingString(delegator, address2); >> - >> String addr2Target >> = PartyWorker.makeMatchingString(delegator, >> address.getString("address2")); >> - >> if (addr2Target != >> null) { >> - >> >> Debug.log("Comparing address2 : " + addr2Source + " / " + >> addr2Target, module); >> - >> - >> if >> (addr2Source.equals(addr2Target)) { >> - >> >> Debug.log("Matching address2; adding valid >> address", module); >> - >> >> validFound.add(address); >> - >> >> //validParty.put(address.getString("partyId"), >> address.getString("contactMechId")); >> - >> } >> - >> } >> - >> } else { >> - >> if >> (address.get("address2") == null) { >> - >> >> Debug.log("No address2; adding valid address", module); >> + // check the address line >> + for (GenericValue address: >> addresses) { >> + // address 1 >> field >> + String >> addr1Source = PartyWorker.makeMatchingString(delegator, >> address1); >> + String >> addr1Target = PartyWorker.makeMatchingString(delegator, >> address.getString("address1")); >> + >> + if (addr1Target >> != null) { >> + >> Debug.log("Comparing address1 : " + addr1Source + " / " + >> addr1Target, module); >> + if >> (addr1Target.equals(addr1Source)) { >> + >> + >> // address 2 field >> + >> if (address2 != null) { >> + >> String addr2Source = >> PartyWorker.makeMatchingString(delegator, address2); >> + >> String addr2Target = >> PartyWorker.makeMatchingString(delegator, >> address.getString("address2")); >> + >> if (addr2Target != null) { >> + >> >> Debug.log("Comparing address2 : " + addr2Source + " / " + >> addr2Target, module); >> + >> + >> if >> (addr2Source.equals(addr2Target)) { >> + >> >> Debug.log("Matching address2; adding valid address", >> module); >> >> >> validFound.add(address); >> >> >> //validParty.put(address.getString("partyId"), >> address.getString("contactMechId")); >> >> } >> >> } >> + >> } else { >> + >> if (address.get("address2") == >> null) { >> + >> Debug.log("No >> address2; adding valid address", module); >> + >> >> validFound.add(address); >> + >> >> //validParty.put(address.getString("partyId"), >> address.getString("contactMechId")); >> + >> } >> >> } >> >> } >> } >> + } >> >> - if >> (UtilValidate.isNotEmpty(validFound)) { >> - >> for (GenericValue partyAndAddr: validFound) { >> - >> String partyId = >> partyAndAddr.getString("partyId"); >> - >> if (UtilValidate.isNotEmpty(partyId)) { >> - >> GenericValue p = >> delegator.findByPrimaryKey("Person", >> UtilMisc.toMap("partyId", partyId)); >> - >> if (p != null) { >> - >> String fName = >> p.getString("firstName"); >> - >> String lName = >> p.getString("lastName"); >> - >> String mName = >> p.getString("middleName"); >> - >> if >> (lName.toUpperCase().equals(lastName.toUpperCase())) { >> - >> if >> (fName.toUpperCase().equals(firstName.toUpperCase())) { >> - >> >> if (mName != null && middleName != >> null) { >> - >> >> if >> (mName.toUpperCase().equals(middleName.toUpperCase())) { >> - >> >> >> returnList.add(partyAndAddr); >> - >> >> } >> - >> >> } else if (middleName == null) { >> + if >> (UtilValidate.isNotEmpty(validFound)) { >> + for >> (GenericValue partyAndAddr: validFound) { >> + >> String partyId = partyAndAddr.getString("partyId"); >> + if >> (UtilValidate.isNotEmpty(partyId)) { >> + >> GenericValue p = >> delegator.findByPrimaryKey("Person", >> UtilMisc.toMap("partyId", partyId)); >> + >> if (p != null) { >> + >> String fName = >> p.getString("firstName"); >> + >> String lName = >> p.getString("lastName"); >> + >> String mName = >> p.getString("middleName"); >> + >> if >> (lName.toUpperCase().equals(lastName.toUpperCase())) { >> + >> if >> (fName.toUpperCase().equals(firstName.toUpperCase())) { >> + >> if >> (mName != null && middleName != null) { >> + >> >> if >> (mName.toUpperCase().equals(middleName.toUpperCase())) { >> >> >> >> returnList.add(partyAndAddr); >> >> >> } >> + >> } >> else if (middleName == null) { >> + >> >> returnList.add(partyAndAddr); >> >> >> } >> >> } >> >> } >> @@ -355,6 +358,7 @@ public class PartyWorker { >> } >> } >> >> + >> return returnList; >> } >> >> >> >> > > > smime.p7s (3K) Download Attachment |
And Asserts right below method definitions rule!
-Adrian --- On Wed, 8/4/10, Scott Gray <[hidden email]> wrote: > From: Scott Gray <[hidden email]> > Subject: Re: svn commit: r982141 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > To: [hidden email] > Date: Wednesday, August 4, 2010, 12:51 AM > Long conditionals are certainly no > fun :-) > > Regards > Scott > > On 4/08/2010, at 7:31 PM, Adrian Crum wrote: > > > Guard blocks are bliss. > > > > -Adrian > > > > --- On Wed, 8/4/10, [hidden email] > <[hidden email]> > wrote: > > > >> From: [hidden email] > <[hidden email]> > >> Subject: svn commit: r982141 - > /ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > >> To: [hidden email] > >> Date: Wednesday, August 4, 2010, 12:27 AM > >> Author: lektran > >> Date: Wed Aug 4 07:27:49 2010 > >> New Revision: 982141 > >> > >> URL: http://svn.apache.org/viewvc?rev=982141&view=rev > >> Log: > >> Replace a long "if not empty" block with an "if > empty" > >> early return, no functional changes > >> > >> Modified: > >> > >> > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > >> > >> Modified: > >> > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java?rev=982141&r1=982140&r2=982141&view=diff > >> > ============================================================================== > >> --- > >> > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > >> (original) > >> +++ > >> > ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java > >> Wed Aug 4 07:27:49 2010 > >> @@ -293,60 +293,63 @@ public class PartyWorker { > >> > >> List<GenericValue> addresses = > >> > EntityUtil.filterByDate(delegator.findList("PartyAndPostalAddress", > >> addrCond, null, sort, null, false)); > >> > >> //Debug.log("Checking for matching > >> address: " + addrCond.toString() + "[" + > addresses.size() + > >> "]", module); > >> > >> + if > >> (UtilValidate.isEmpty(addresses)) { > >> + // No > address > >> matches, return an empty list > >> + return > >> returnList; > >> + } > >> + > >> > >> List<GenericValue> validFound > = > >> FastList.newInstance(); > >> - if > >> (UtilValidate.isNotEmpty(addresses)) { > >> - // > check the > >> address line > >> - for > >> (GenericValue address: addresses) { > >> - > // > >> address 1 field > >> - > > >> String addr1Source = > >> PartyWorker.makeMatchingString(delegator, > address1); > >> - > > >> String addr1Target = > >> PartyWorker.makeMatchingString(delegator, > >> address.getString("address1")); > >> - > >> - > if > >> (addr1Target != null) { > >> - > > >> Debug.log("Comparing > address1 : " + > >> addr1Source + " / " + addr1Target, module); > >> - > > >> if > (addr1Target.equals(addr1Source)) { > >> - > >> - > > >> // address 2 > field > >> - > > >> if (address2 > != null) { > >> - > > >> > String addr2Source > >> = PartyWorker.makeMatchingString(delegator, > address2); > >> - > > >> > String addr2Target > >> = PartyWorker.makeMatchingString(delegator, > >> address.getString("address2")); > >> - > > >> > if (addr2Target != > >> null) { > >> - > > >> > > >> Debug.log("Comparing address2 : " + addr2Source + > " / " + > >> addr2Target, module); > >> - > >> - > > >> > if > >> (addr2Source.equals(addr2Target)) { > >> - > > >> > > >> Debug.log("Matching > address2; adding valid > >> address", module); > >> - > > >> > > >> validFound.add(address); > >> - > > >> > > >> > //validParty.put(address.getString("partyId"), > >> address.getString("contactMechId")); > >> - > > >> > } > >> - > > >> > } > >> - > > >> } else { > >> - > > >> > if > >> (address.get("address2") == null) { > >> - > > >> > > >> Debug.log("No address2; adding valid address", > module); > >> + // check the address > line > >> + for (GenericValue > address: > >> addresses) { > >> + // > address 1 > >> field > >> + String > >> addr1Source = > PartyWorker.makeMatchingString(delegator, > >> address1); > >> + String > >> addr1Target = > PartyWorker.makeMatchingString(delegator, > >> address.getString("address1")); > >> + > >> + if > (addr1Target > >> != null) { > >> + > > >> Debug.log("Comparing address1 : " + addr1Source + > " / " + > >> addr1Target, module); > >> + > if > >> (addr1Target.equals(addr1Source)) { > >> + > >> + > > >> // address 2 field > >> + > > >> if (address2 != null) { > >> + > > >> String > addr2Source = > >> PartyWorker.makeMatchingString(delegator, > address2); > >> + > > >> String > addr2Target = > >> PartyWorker.makeMatchingString(delegator, > >> address.getString("address2")); > >> + > > >> if > (addr2Target != null) { > >> + > > >> > >> Debug.log("Comparing address2 : " + addr2Source + > " / " + > >> addr2Target, module); > >> + > >> + > > >> > if > >> (addr2Source.equals(addr2Target)) { > >> + > > >> > > >> Debug.log("Matching address2; adding valid > address", > >> module); > >> > > >> > >> validFound.add(address); > >> > > >> > >> > //validParty.put(address.getString("partyId"), > >> address.getString("contactMechId")); > >> > > >> } > >> > > >> } > >> + > > >> } else { > >> + > > >> if > (address.get("address2") == > >> null) { > >> + > > >> > Debug.log("No > >> address2; adding valid address", module); > >> + > > >> > >> validFound.add(address); > >> + > > >> > >> //validParty.put(address.getString("partyId"), > >> address.getString("contactMechId")); > >> + > > >> } > >> > > >> } > >> > >> } > >> } > >> + } > >> > >> - if > >> (UtilValidate.isNotEmpty(validFound)) { > >> - > > >> for (GenericValue partyAndAddr: validFound) { > >> - > > >> String partyId = > >> partyAndAddr.getString("partyId"); > >> - > > >> if > (UtilValidate.isNotEmpty(partyId)) { > >> - > > >> GenericValue > p = > >> delegator.findByPrimaryKey("Person", > >> UtilMisc.toMap("partyId", partyId)); > >> - > > >> if (p != > null) { > >> - > > >> > String fName = > >> p.getString("firstName"); > >> - > > >> > String lName = > >> p.getString("lastName"); > >> - > > >> > String mName = > >> p.getString("middleName"); > >> - > > >> > if > >> > (lName.toUpperCase().equals(lastName.toUpperCase())) { > >> - > > >> > if > >> > (fName.toUpperCase().equals(firstName.toUpperCase())) { > >> - > > >> > > >> if (mName != null > && middleName != > >> null) { > >> - > > >> > > >> if > >> > (mName.toUpperCase().equals(middleName.toUpperCase())) { > >> - > > >> > > >> > >> returnList.add(partyAndAddr); > >> - > > >> > > >> } > >> - > > >> > > >> } else if (middleName == > null) { > >> + if > >> (UtilValidate.isNotEmpty(validFound)) { > >> + for > >> (GenericValue partyAndAddr: validFound) { > >> + > > >> String partyId = > partyAndAddr.getString("partyId"); > >> + > if > >> (UtilValidate.isNotEmpty(partyId)) { > >> + > > >> GenericValue p = > >> delegator.findByPrimaryKey("Person", > >> UtilMisc.toMap("partyId", partyId)); > >> + > > >> if (p != null) { > >> + > > >> String fName > = > >> p.getString("firstName"); > >> + > > >> String lName > = > >> p.getString("lastName"); > >> + > > >> String mName > = > >> p.getString("middleName"); > >> + > > >> if > >> > (lName.toUpperCase().equals(lastName.toUpperCase())) { > >> + > > >> > if > >> > (fName.toUpperCase().equals(firstName.toUpperCase())) { > >> + > > >> > if > >> (mName != null && middleName != null) { > >> + > > >> > > >> if > >> > (mName.toUpperCase().equals(middleName.toUpperCase())) { > >> > > >> > > >> > >> returnList.add(partyAndAddr); > >> > > >> > > >> } > >> + > > >> > } > >> else if (middleName == null) { > >> + > > >> > > >> > returnList.add(partyAndAddr); > >> > > >> > >> } > >> > > >> } > >> > > >> } > >> @@ -355,6 +358,7 @@ public class PartyWorker { > >> } > >> } > >> > >> + > >> return > returnList; > >> } > >> > >> > >> > >> > > > > > > > > |
In reply to this post by Scott Gray-2
Scott Gray wrote:
> Long conditionals are certainly no fun :-) Yes, I like this pattern. Except for certain code checkers that complain about multiple return paths from a method. |
On 5/08/2010, at 2:08 AM, Adam Heath wrote:
> Scott Gray wrote: >> Long conditionals are certainly no fun :-) > > Yes, I like this pattern. Except for certain code checkers that > complain about multiple return paths from a method. You can please some of the code checkers some of the time but you can't please all of the code checkers all of the time. Regards Scott smime.p7s (3K) Download Attachment |
Free forum by Nabble | Edit this page |