I want to update an object, but beforehand I check the fields that can be updated; then I collect the fields' names to a list.

My problem is that it has too many if-statements, and I want to reduce them. Is there any nice way to do it?

/**
 * This method checks if there are any updates for a company
 *
 * @param company The data that comes from the database, and we want to update.
 * @param companyDTO The data that may contain updates.
 * @return The method returns with a list that contains the changes.
 */
private List<String> getChanges(Company company, CompanyDTO companyDTO) {
    List<String> changes = new ArrayList<>();

    if (companyDTO.getName() != null && !companyDTO.getName().equals(company.getName())) {
        changes.add(COMPANY_NAME);
    }
    if (companyDTO.getShortName() != null && !companyDTO.getShortName().equals(company.getShortName())) {
        changes.add(COMPANY_SHORTNAME);
    }
    if (companyDTO.getAddress() != null && !companyDTO.getAddress().equals(company.getAddress())) {
        changes.add(COMPANY_ADDRESS);
    }
    if (companyDTO.getTaxNumber() != null && !companyDTO.getTaxNumber().equals(company.getTaxNumber())) {
        changes.add(COMPANY_TAX_NUMBER);
    }
    if (companyDTO.getFirmId() != null && !companyDTO.getFirmId().equals(company.getFirmId())) {
        changes.add(COMPANY_FIRM_ID);
    }
    if (companyDTO.getBankName() != null && !companyDTO.getBankName().equals(company.getBankName())) {
        changes.add(COMPANY_BANK_NAME);
    }
    if (companyDTO.getBankAccountNumber() != null && !companyDTO.getBankAccountNumber().equals(company.getBankAccountNumber())) {
        changes.add(COMPANY_BANK_ACCOUNT_NUMBER);
    }
    if (companyDTO.getInternalComment() != null && !companyDTO.getInternalComment().equals(company.getInternalComment())) {
        changes.add(COMPANY_INTERNAL_COMMENT);
    }
    if (companyDTO.getEnabled() != null && !companyDTO.getEnabled().equals(company.getEnabled())) {
        changes.add(ENABLED);
    }
    return changes;
}

CodePudding user response：

I see two general approaches:

1. create objects encapsulating the varying part of the if statements:

   record ChangeCheck(Function<CompanyDTO, Object> dtoField, Function<Company, Object> entityField, Change change){
   
       boolean check (CompanyDTO dto, Company entity){
           return Objects.equals(dtoField.apply(dto), entityField.apply(company);
       }
   }
   

   You can then create a collection of such objects

   List<ChangeCheck> checkers = new ArrayList<>();
   
   checkers.add(new ChangeCheck(CompanyDTO::getBankName, Company::getBankName, COMPANY_BANK_NAME));
   

   In your method you just iterate of that collection.

   for (ChangeCheck check : checkers){
       if (check.check(companyDTO, company)) {
           changes.add(check.check);
       }
   }
   

2. Alternatively you can use reflection to find all the fields of the dto, the matching fields of the entity and the constant/enum whatever to add to your collection of changes.

The first approach is easy to implement and to understand, but requires about the same amount of typing as the if conditions. At least it does remove the duplication of logic.

CodePudding user response：

You could extract those calls that get the value into a (lambda) function, then use a list of those functions and iterate them, roughly like

@Value // lombok to write the constructor, write out the class if you don't have that
static class Extractor {
    final Function<CompanyDTO, Object> dtoValue;
    final Function<Company, Object> value;
    String name;
}

List<Extractor> extractors = Arrays.asList(
        new Extractor(CompanyDTO::getA, Company::getA, "company.a"),
        new Extractor(CompanyDTO::getB, Company::getB, "company.b")
);

private List<String> getChanges(Company company, CompanyDTO companyDTO) {
    List<String> result = new ArrayList<>();
    extractors.forEach(e -> {
        Object old = e.value.apply(company);
        Object updated = e.dtoValue.apply(companyDTO);
        if (old != null && !old.equals(updated))
            result.add(e.name);
    });
    return result;
}

@Value
static class CompanyDTO {
    final String a;
    final Date b;
}

@Value
static class Company {
    final String a;
    final Date b;
}

CodePudding user response：

You could create three Lists, one of the objects you want to compare, one with the other objects you want to compare, and a third one with all the constants:

private List<String> getChanges(Company company, CompanyDTO companyDTO) {
   List<String> changes = new ArrayList<>();

   Object[] first = [company.getName(), company.getCity(), company.getCountry() ...];
   Object[] second = [companyDTO.getName(), companyDTO.getCity(), companyDTO.getCountry() ...];
   String[] constants = [COMPANY_NAME, COMPANY_CITY, COMPANY_COUNTRY ...];

   for (int i = 0; i < first.length; i  ) {
      if (first != null && !first[i].equals(second[i])) {
         changes.add(constants[i]);
      }
   }

   return changes;
}

CodePudding user response：

I'll say use Reflection here.

Assuming Company & CompanyDTO have same field names.

private List<String> getChanges(final Company company, final CompanyDTO companyDTO) {
     final List<String> result = new ArrayList<>();
     for (final Field dtoField : CompanyDTO.class.getDeclaredFields()) {
          ReflectionUtils.makeAccessible(dtoField);
          for (final Field comField : Company.class.getDeclaredFields()) {
               ReflectionUtils.makeAccessible(comField);
               if (dtoField.getName().equals(comField.getName())) {
                    final Object dtoObject = ReflectionUtils.getField(dtoField, dto);
                    final Object companyObject = ReflectionUtils.getField(comField, com);
                    if (!Objects.equals(dtoObject, companyObject)) {
                         result.add(dtoField.getName());
                    }
               }
          }
     }
     return result;
}