-3

I have created a basic CRUD API using Spring Boot , in that I have created a service class for my controller.

The following is my service method of the Controller.

Services

public Customer updateCustomer(Customer newCustomer, Long customerId) throws ResourceNotFoundException {
        
        return customerRepo.findById(customerId)
                .map(customer -> {
                    if (newCustomer.getName() != null)
                        customer.setName(newCustomer.getName());
                    if (newCustomer.getGstin() != null)
                        customer.setGstin(newCustomer.getGstin());
                    if (newCustomer.getPhoneNumber() != null)
                        customer.setPhoneNumber(newCustomer.getPhoneNumber());
                    if (newCustomer.getAddress() != null)
                        customer.setAddress(newCustomer.getAddress());
                    if (newCustomer.getOutstandingBalance() != 0.0f)
                        customer.setOutstandingBalance(newCustomer.getOutstandingBalance());
                    return customerRepo.save(customer);
                }).orElseThrow(() -> new ResourceNotFoundException());

    }

My question is: Is it possible to simplify the code which is Using multiple if?

If there is, can anyone suggest a simplification to handle this logic..??

Lajos Arpad
  • 64,414
  • 37
  • 100
  • 175
ashil
  • 99
  • 11
  • 1
    if you care about non overwriting a property already set, use the `if`. I you can set to null a property which is already null, remove the `if`s – fantaghirocco Jan 07 '21 at 10:53
  • 1
    As far as the Java code is concerned, there's no other construct that could perform this logical sequence of checking and setting any better. – Thomas Timbul Jan 07 '21 at 10:56
  • 1
    You could use Something like Optional.of(newCustomer.getName).ifPresent(String s -> customer.setName(s)), so that you avoid all this if, and the code is more clear. – Fabio Piunti Jan 07 '21 at 10:58

3 Answers3

3

Yet another approach of doing that might look like this.

I would love if Java supported in-language macros so that such patterns might be wrapped in a macro call and then expanded to generated source code. Having no macros, that all can be put in methods and do the check-copy pattern at runtime:

public final class Patch {

    private Patch() {
    }

    public static <T> void nonNull(final Supplier<? extends T> get, final Consumer<? super T> set) {
        @Nullable
        final T value = get.get();
        if ( value != null ) {
            set.accept(value);
        }
    }

    public static void nonZero(final DoubleSupplier get, final DoubleConsumer set) {
        final double value = get.getAsDouble();
        if ( value != 0 ) {
            set.accept(value);
        }
    }

}

Test:

@Data
@AllArgsConstructor
final class Customer {

    private final String name;
    private String phoneNumber;
    private float outstandingBalance;

}

And the test:

@Test
public void test() {
    final Customer customer = new Customer(null, "+123", 2);
    final Customer newCustomer = new Customer("john-doe", null, 0);
    Patch.nonNull(customer::getName, newCustomer::setName);
    Patch.nonNull(customer::getPhoneNumber, newCustomer::setPhoneNumber);
    Patch.nonZero(customer::getOutstandingBalance, value -> newCustomer.setOutstandingBalance((float) value));
    Assertions.assertEquals(new Customer("john-doe", "+123", 2), newCustomer);
}

Note that tools like Lombok generate Java code at compile time, but being javac plugins (see the @Data and AllArgsConstructor annotations) are black boxes that generate code themselves (equals(), hashCode(), toString(), get***() and set***() methods + the all-args constructor for Customer respectively) limiting to their current features only (+ requires IDE support to make it look nicer).

If you prefer Java Reflection that (may have) has higher runtime cost, take a look at Apache Commons BeanUtils that seem to do that job at runtime: Helper in order to copy non null properties from object to another


By the way, don't use float for outstandingBalance -- use BigDecimal or whatever precise. Why not use Double or Float to represent currency?

1

It's not,what i suggest is that the front-end/client that is sending the Customer to send as a complete Customer and your method just does the save.

The one you used will work fine but using multiple if statements is not advised even the IDE gives warnings at least a good ide should.

Walter
  • 11
  • 2
  • 1
    The question was asked in an opinionated mode. I have edited it, so it is much more objective now. The asker was wondering whether the code can be simplified, so as to avoid the repetition of the if-then pattern. You are right when you say that this is not the best approach, but epistemologically speaking you can only know that if you know a better approach. However, your answer does not contain an alternative approach. I suggest the use of reflection. If a method is implementing that pattern, getting the customer and newCustomer and an array of names, then that can be used for simplification. – Lajos Arpad Jan 07 '21 at 11:45
  • 1
    Because in that case you would be able to only pass some parameters and let the logic to be unfolded under the hoods. Much better than the pattern repetition we have seen in the question. In short, I agree with you, but I cannot upvote your answer unless you provide a solution as well. – Lajos Arpad Jan 07 '21 at 11:47
1

Yes, there is such a simplification. Using reflection, you can call a method of a class/instance. Assuming that you have a method that takes String[] as parameter, containing values like "Name", you can iterate that array and invoke newCustomer's getter, compare it to null and if it's not null, then you can invoke customer's setter, passing the result of the getter you just invoked.

See this post for more information: https://www.baeldung.com/java-method-reflection

Lajos Arpad
  • 64,414
  • 37
  • 100
  • 175
  • Using reflection here would make that code west-wild. What if the OP has more than 5 properties in their bean class and not all of them are supposed to be checked and copied? – terrorrussia-keeps-killing Jan 07 '21 at 11:44
  • @fluffy indeed, there could be multiple problems plaguing that project. However, the op has asked about a single pattern and this answer is aiming to cope with that. I recommend the idea represented in this answer to be used for the problem presented in the question and I recommend not adhering to this answer for problems that are very different from the one presented in the question. – Lajos Arpad Jan 08 '21 at 09:33