0

Writing a code for checking data on different conditions. I have an ArrayList of CRM objects "actionsList" (made in another class). Here I check these objects for different conditions. Objects which satisfy the conditions I have to add to the ArrayList "remarksList". The question is how to create a method setRemarkObject() to set all data to remarkObject at once? Not to write each time:

  remarkObject.setRemark(checkString);
  remarkObject.setNumber(crm.getNumber());
  remarkObject.setDealer(crm.getDealer());
  remarkObject.setName(crm.getName());

...

is it correct now?

import java.util.ArrayList;

public class Conditions {

    static ArrayList<CRM> remarksList = new ArrayList<CRM>();

    public ArrayList<CRM> conditionsChecking() {
        for (CRM crm : App.actionsList) {
            CRM remarkObject = new CRM();

            remarkObject.setNumber(crm.getNumber());
            remarkObject.setDealer(crm.getDealer());
            remarkObject.setName(crm.getName());
            remarkObject.setGroup(crm.getGroup());
            remarkObject.setClientStatus(crm.getClientStatus());
            remarkObject.setEntity(crm.getEntity());
            remarkObject.setTypeOfContact(crm.getTypeOfContact());
            remarkObject.setTypeOfFirstContact(crm.getTypeOfFirstContact());
            remarkObject.setSourceOfFirstContact(crm
                    .getSourceOfFirstContact());
            remarkObject.setOfferType(crm.getOfferType());
            remarkObject.setEventDate(crm.getEventDate());
            remarkObject.setBrand(crm.getBrand());
            remarkObject.setCarClass(crm.getCarClass());
            remarkObject.setModel(crm.getModel());
            remarkObject.setCarCode(crm.getCarCode());
            remarkObject.setWeek(crm.getWeek());
            remarkObject.setMonth(crm.getMonth());
            remarkObject.setYear(crm.getYear());
            remarkObject.setAmmount(crm.getAmmount());
            remarkObject.setSalesman(crm.getSalesman());
            remarkObject.setPhone(crm.getPhone());
            remarkObject.setEmail(crm.getEmail());
            remarkObject.setAddress(crm.getAdress());
            remarkObject.setCreationDate(crm.getCreationDate());
            remarkObject.setCreationTime(crm.getCreationTime());
            remarkObject.setModificationDate(crm.getModificationDate());
            remarkObject.setModificationTime(crm.getModificationTime());
            remarkObject.setBackdating(crm.getBackdating());

            if ((crm.getClientStatus().equals("Yes")) && ((crm.getAdress().isEmpty()))){
                crm.setRemark("Client's address is empty");
            remarksList.add(remarkObject);
            }
            else if ((crm.getClientStatus().equals("Yes")) && (crm.getPhone().isEmpty())){          
                crm.setRemark( "Phone field is empty");
            remarksList.add(remarkObject);
            }
            ///....
            else
                crm.setRemark("Nothing wrong");
                /// not adding to remarksLis

        }
        return remarksList;
    }

}

2 Answers2

0

You seem to be doing the same thing over and over again, with the addition of a String which states if a field is left empty. So Why not move the setters one block up, outside of the for loop?

static ArrayList<CRM> remarksList = new ArrayList<CRM>();

public ArrayList<CRM> conditionsChecking() {
    for (CRM crm : App.actionsList) {
        CRM remarkObject = new CRM();

        remarkObject.setNumber(crm.getNumber());
        remarkObject.setDealer(crm.getDealer());
        remarkObject.setName(crm.getName());
        remarkObject.setGroup(crm.getGroup());
        //etcetera, etcetera, all setters, except remark

        if ((crm.getClientStatus().equals("Yes")) && ((crm.getAdress().isEmpty())))
            crm.setRemark("Client's address is empty");
        else if ((crm.getClientStatus().equals("Yes")) && (crm.getPhone().isEmpty()))            
            crm.setRemark( "Phone field is empty");
        // etc, etc
        else
            remarksList.add(remarkObject);
    }
    return remarksList;
}

For setters and getters in general, you can also use a constructor:

public CRM(String number, String dealer, String name, ...) { 
    this.number = number;
    //...etc
}

Or a builder

public CRM() {}

public CRM setNumber(String number) { 
    this.number = number;
    return this;
}

public CRM setDealer(String dealer) { 
    this.dealer = dealer;
    return this;
}

//and use it like this
CRM crm = new CRM().setNumber("123").setDealer("dealer"); //et cetera
stealthjong
  • 10,858
  • 13
  • 45
  • 84
  • Cannot move setters up because I should add to remarksList not all the objects from actionsList, but only that ones which satisfy conditions. –  Aug 15 '14 at 08:34
  • Or will it be correctly to add all the objects to remarksList (or even leave the actionsList itself), add conditions like if(...) {testString = "Bla bla bla";} elle {testString = null;}. And then just delete all the objects with testString = null?? –  Aug 15 '14 at 08:41
  • @Leonid, no, see edit. I'd check if remark is set or not. `if (crm.getRemark() != null) remarksList.add(remarkObject)` – stealthjong Aug 15 '14 at 08:50
  • I guess I've got it. Thanks a lot. Getting your first example except one thing - will add to to remarksList just objects, satisfied if and else if –  Aug 15 '14 at 08:52
  • is it correct to add objects to the List in if (like I've edited), else if statements or I'd better use if (crm.getRemark() != null) remarksList.add(remarkObject)? –  Aug 15 '14 at 09:05
  • @Leonid if you do it in every if/ifelse, it's a few more statements. It doesnt matter all that much, though – stealthjong Aug 15 '14 at 09:08
0

From what I understand you are copying the crm object into the remarkObject in every if-else statements. Why not copy it before all the if statements and only set the remark(checkString) inside the if-else statements? Then you can add it to remarkList after all of those statements. You can copy the object using CRM constructor ie. CRM(CRM crm) or by cloning the object: How do I copy an object in Java?

Also you are checking crm.getClientStatus().equals("Yes") in every of your if statement - why not check it once at a begining?

Community
  • 1
  • 1
Michał Schielmann
  • 1,372
  • 8
  • 17
  • I have got initial ArrayList "actionsList". It contains all the data from Excel file (not small). Here I have to put to the remarksList just incorrect statements (that not satisfy conditions. –  Aug 15 '14 at 08:24