Populate errors for objects

I have a map of objects and errors associated with those objects.

I have to do the following:

• populate the errors in the object

-
populate errors for valid objects based on the following:

• check the indicator property in the object ( Y | N | other )

• if all have Y and if there are invalid records - need to reject all

Scenario 1: if there are records with different indicators, then get all the valid records if any ( i.e., no errors associated with them ) and set the error description to inconsistent indicator.

Scenario 2: if all the records have indicator as Y and if there are any invalid records then get all the valid records if any and set the error description to rejected because indicator is set to Y.

The following is a working code. How can I improve this further? I feel like I am looping the list more times than required.

```
private List populateBeansWithErrors(final Map> errorsMap) throws FinancialsSystemRuntimeException {
final List someObjs = new ArrayList<>();

for ( final Entry> entry : errorsMap.entrySet() )
{
SomeClazz someObj = entry.getKey();
final Errors errors = entry.getValue();
populateErrorColumns(someObj, errors);
someObjs.add(someObj);
}

checkAndSetErrorColumnsForValidRecords(someObjs);

return someObjs;
}

private void checkAndSetErrorColumnsForValidRecords(final List someObj) {

//split list into sub-lists based on the indicator.
final List beansWithIndicatorY = new ArrayList<>();
final List beansWithIndicatorN = new ArrayList<>();
final List beansWithIndicatorNotValid = new ArrayList<>();

filterListPredicatesOnAllOrNoneIndicator(someObj, beansWithIndicatorY, beansWithIndicatorN, beansWithIndicatorNotValid);

String errorDescription = IndicatorValidator.INCONSISTENT_INDICATOR;

//if there are records with different indicators
//then get all the valid records

Just a few generic notes:

-
I guess you're using SLF4J here:

Logger.info("Validation errors exist in the file for row {}", someObj.toString());

The toString() call is unnecessary, SLF4J will call it for you (and it will call only if you have enabled info level logging).

Logger.info("Validation errors exist in the file for row {}", someObj);

-
I wouldn't go with size() here:

if(beansWithIndicatorN.size() > 0) {

!isEmpty would be a little bit easier to read and closer to English:

if (!beansWithIndicatorN.isEmpty()) {

-
For naming here:

private List populateBeansWithErrors(final Map> errorsMap)
        throws FinancialsSystemRuntimeException {
    final List someObjs = new ArrayList<>();
    ...
    return someObjs;
}

I'd rename to someObjs to result to express its purpose.

-
The name of the following variables are too similar to each other:

beansWithIndicatorY
beansWithIndicatorN
beansWithIndicatorNotValid

I'd try to choose something which is easier to tell each other apart, like: yesBeans, noBeans, invalidBeans.

-
I supposed that the someObj parameter should be someObjs:

private void filterListPredicatesOnAllOrNoneIndicator(
        @NotNull final List someObj,
        @NotNull final List beansWithIndicatorY,
        @NotNull final List beansWithIndicatorN,
        @NotNull final List beansWithIndicatorNotValid) {

    for(SomeClazz someObj : someObj) {
        if(StringUtils.equalsIgnoreCase(SomeClazz.getAllOrNoneIndicator(), INDICATOR_IS_Y)) {
            beansWithIndicatorY.add(someObj);
        } else if(StringUtils.equalsIgnoreCase(SomeClazz.getAllOrNoneIndicator(), INDICATOR_IS_N)) {
            beansWithIndicatorN.add(someObj);
        } else {
            beansWithIndicatorNotValid.add(someObj);
        }
    }
}

Are you sure that the loop is correct here? The inner conditions don't depend on the iterated value, so if SomeClazz.getAllOrNoneIndicator() doesn't change on its subsequent calls, the following is the same:

private void filterListPredicatesOnAllOrNoneIndicator(
        @NotNull final List someObjs,
        @NotNull final List beansWithIndicatorY, 
        @NotNull final List beansWithIndicatorN,
        @NotNull final List beansWithIndicatorNotValid) {

    if (StringUtils.equalsIgnoreCase(SomeClazz.getAllOrNoneIndicator(), INDICATOR_IS_Y)) {
        beansWithIndicatorY.addAll(someObjs);
    } else if (StringUtils.equalsIgnoreCase(SomeClazz.getAllOrNoneIndicator(), INDICATOR_IS_N)) {
        beansWithIndicatorN.addAll(someObjs);
    } else {
        beansWithIndicatorNotValid.addAll(someObjs);
    }
}

StackExchange Code Review Q#44999, answer score: 4