update object in list Q: I have a Bank class containing multiple loan accounts (LoanAccount class). I've create a LoanAccountService that have the CRUD functionalities. My concerns are about how I implemented the update functionality.


Bank

public class Bank {
    private List&lt;LoanAccount&gt; loanAccounts;
}


Loan account

public class LoanAccount {
    private String id;
    private Integer numberOfInstallments;
    private LoanAccountType type;
    private Date creationDate;
    private BigDecimal loanAmount;
}


Service

public class LoanAccountService{

    private Bank bank;

    public LoanAccountService(Bank bank) {
        this.bank = bank;
    }

    public LoanAccount update(LoanAccount loanAccount) {
        Optional&lt;LoanAccount&gt; account = bank.getLoanAccounts()
                .stream()
                .filter(la -&gt; la.getId().equals(loanAccount.getId()))
                .findAny();
        if (account.isPresent()) {
            account.get().setCreationDate(loanAccount.getCreationDate());
            account.get().setLoanAmount(loanAccount.getLoanAmount());

        account.get().setNumberOfInstallments(loanAccount.getNumberOfInstallments());
            account.get().setType(loanAccount.getType());
        } else {
            throw new IllegalArgumentException("The object does not exist.");
        }
        return loanAccount;
    }
}


When the method update is called with a LoanAccount containing an id that already exists in loanAccounts list, I want to update the existing object with the object loanAccount given as parameter.

Above is my implementation, but I feel like there should be better ways to do it.
I would suggest following:


Place operations to the objects that contain data for this operation. It will improve encapsulation and allow more flexible code reuse.
Also it will be easy to test such implementation because of small methods.  
Use more convenient structure. In this case Map&lt;String, LoanAccount&gt; (map id to loanAccount) instead of List&lt;LoanAccount&gt;


As you can see in this case service contains only required logic (how to react on loan absence). It is easy to read, test and understand.

LoanAccountService.java

public class LoanAccountService{

    private Bank bank;

    public LoanAccountService(Bank bank) {
        this.bank = bank;
    }

    public LoanAccount update(LoanAccount loanAccount) {
        if (!bank.updateLoanAccount(loanAccount)) {
            throw new IllegalArgumentException("The object does not exist.");
        }
        return loanAccount;
    }
}


Bank.java

@Getter
@Setter
public class Bank {
    private Map&lt;String, LoanAccount&gt; loanAccounts;

    public boolean updateLoanAccount(LoanAccount loanAccount) {
        LoanAccount loan = loanAccounts.get(loanAccount.getId());
        if (loan != null) {
            loan.update(loanAccount);
            return true;
        }
        return false;
    }
}


LoanAccount.java

@Getter
@Setter
public class LoanAccount {
    private String id;
    private Integer numberOfInstallments;
    private LoanAccountType type;
    private Date creationDate;
    private BigDecimal loanAmount;

    public void update(LoanAccount loanAccount) {
        this.setCreationDate(loanAccount.getCreationDate());
        this.setLoanAmount(loanAccount.getLoanAmount());
        this.setNumberOfInstallments(loanAccount.getNumberOfInstallments());
        this.setType(loanAccount.getType());
    }
}


If you can't use this approach, you could made a small refactoring of the service (less code and "if conditions" are good things):

public LoanAccount update(LoanAccount loanAccount) {
    LoanAccount account = bank.getLoanAccounts()
            .stream()
            .filter(la -&gt; la.getId().equals(loanAccount.getId()))
            .findAny()
            .orElseThrow(() -&gt;  new IllegalArgumentException("The object does not exist."));
    account.setCreationDate(loanAccount.getCreationDate());
    account.setLoanAmount(loanAccount.getLoanAmount());
    account.setNumberOfInstallments(loanAccount.getNumberOfInstallments());
    account.setType(loanAccount.getType());
    return account;
}


Also I think you should return updated loan account.