Mapper and service call I have an entity called Travel which has list of costCenter. I have created a code for handling CRUD operation for cost centers

protected void assignCostCenter(CostCenterDTO costCenterDTO, Travel travel, Long detailsId) throws BusinessException {
    CostCenter costCenter = null;
    if (costCenterDTO.getCostCenters() != null) {
        Center center = commonDao.find(costCenterDTO.getCostCenters().getId());
        costCenter = costCenterDTOMapper.mapToDTO(costCenterDTO.getCostCenters(), travel, detailsId, center);
        if (costCenter != null &amp;&amp; !travel.getCostCenters().contains(costCenter)) {
            travel.getCostCenters().add(costCenter);
        }
    } else {
        travel.setCostCenters(travel.getCostCenters().stream().filter(costCenter1 -&gt; !(costCenter1.getDetailId().equals(detailsId))).collect(Collectors.toList()));
    }
}


And in the mapper 

 public CostCenter mapToCostCenter(CostCenterDTO costCenterDTO, Travel travel, Long claimDetail, Center center) throws BusinessException {
        CostCenter costCenter = new CostCenter();
        CostCenter costCenterToUpdate = travel.getCostCenters()
                .stream()
                .filter(costCenter1 -&gt; costCenter1.getDetailId().equals(claimDetail))
                .findFirst()
                .orElse(null);
        if (costCenterToUpdate == null) {
            costCenter.setCenter(center);
            costCenter.setTravel(travel);
            costCenter.setDetailId(claimDetail);
        } else {
            costCenterToUpdate.setCenter(center);
            return costCenterToUpdate;
        }
        return costCenter;
    }


The functionality is that if new CostCenter is assigned, I fetch Center from db which was assigned and pass to mapper. In mapper I call travel.getCostCenters to see if there is no costCenter already assigned with same detail id if there is I just update it, if there is no I create new and return, and add to list if doesn't exist. Also if CostCenterDTO comes as null then I remove from list costCenter by detailId. My question is how I could refactor it because it doesn't look tidy?
I think the mapper looks ok, but in the cost centers management method, you violate encapsulation by directly manipulating the travel's cost centers list.

I'd rather leave the list management to the travel class:

class Travel {
    ...
    void addCostCenter(CostCenter c) {
       ...
    }

    void removeCostCenter(CostCenter c) {
       ...
    }

    boolean containsCostCenter(CostCenter c) {
        ...
    }

    CostCenter findByDetails(Long detailsId) {
        ....
    }
}


... and use it like this:

protected void assignCostCenter(CostCenterDTO costCenterDTO, Travel travel, Long detailsId) throws BusinessException {
    // note: moved outside declaration of CostCenter to smaller scope
    if (costCenterDTO.getCostCenters() != null) {
        Center center = commonDao.find(costCenterDTO.getCostCenters().getId());
        CostCenter costCenter = costCenterDTOMapper.mapToDTO(costCenterDTO.getCostCenters(), travel, detailsId, center);
        // can costCenterDTOMapper.mapToDTO actually return null? If not, declare it like that and
        // remove the ugly null-check
        if (costCenter != null &amp;&amp; !travel.containsCostCenter(costCenter)) {
            // depending on the business logic, you might also consider
            //    travel.addCostCenterIfNotExists(costCenter)
            // and skip the if
            travel.addCostCenter(costCenter);
        }
    } else {
        travel.removeCostCenter(travel.findByDetails(detailsId));
    }
}