CODEWITHSUNDEEP
javaspringrefactoring
elena A
elena A

Reputation: 35

Refactoring a Java method so that the code won't be marked as duplicate

I have a project that is using Spring boot and Spring Data JPA interfaces. There is a method which appears in 5 services and the code is being marked by my IDE as duplicate. I will post 2 of the methods so you can see how the code looks like, it's basically the same, the only thing that differs is the repository interface used.

In AccountRequestService.java I have this method:


    public Page<AccountRequest> getAccountRequestPageable(final Pageable pageable, final String status) {

        Page<AccountRequest> accountRequestPage;

        if (UNCOMPLETED_STATUS.equals(status)) {
            accountRequestPage = accountRequestRepository.findByBaseRequestInitiatorIgnoreCaseAndBaseRequestOutdatedDaysGreaterThanAndBaseRequestStatus(pageable,
                    LoggedUserUtil.getLoggedUserName(), 0, BaseRequestStatusType.APPROVED);
        } else {
            if (!ALL_STATUS.equals(status)) {
                accountRequestPage = accountRequestRepository.findByBaseRequestStatusAndBaseRequestInitiatorIgnoreCase(pageable,
                        BaseRequestStatusType.valueOf(status), LoggedUserUtil.getLoggedUserName());
            } else {
                accountRequestPage = accountRequestRepository.findByBaseRequestInitiatorIgnoreCase(pageable, LoggedUserUtil.getLoggedUserName());
            }
        }
        return accountRequestPage;
    }

In ApplicationRequestService.java:

  public Page<ApplicationsRequest> getApplicationsRequestPageable(final Pageable pageable, final String status) {
        Page<ApplicationsRequest> applicationsRequestPage;
        if (UNCOMPLETED_STATUS.equals(status)) {
            applicationsRequestPage = applicationRequestRepository
                    .findByBaseRequestInitiatorIgnoreCaseAndBaseRequestOutdatedDaysGreaterThanAndBaseRequestStatus(pageable, LoggedUserUtil.getLoggedUserName(), 0,
                            BaseRequestStatusType.APPROVED);
        } else {
            if (!ALL_STATUS.equals(status)) {
                applicationsRequestPage = applicationRequestRepository.findByBaseRequestStatusAndBaseRequestInitiatorIgnoreCase(
                        pageable, BaseRequestStatusType.valueOf(status), LoggedUserUtil.getLoggedUserName());
            } else {
                applicationsRequestPage = applicationRequestRepository.findByBaseRequestInitiatorIgnoreCase(pageable, LoggedUserUtil.getLoggedUserName());
            }
        }
        return applicationsRequestPage;
    }

I will post the code for AccountRequestRepository, as ApplicationRequestRepository is similar, but refers the object type ApplicationRequest instead of AccountRequest:

public interface AccountRequestRepository extends JpaRepository<AccountRequest, Long> {
    AccountRequest findByBaseRequestId(Long id);

    Page<AccountRequest> findByBaseRequestInitiatorIgnoreCaseAndBaseRequestOutdatedDaysGreaterThanAndBaseRequestStatus(Pageable pageable, String initiator,
                                                                                                                       Integer days, BaseRequestStatusType status);

    Page<AccountRequest> findByBaseRequestStatusAndBaseRequestInitiatorIgnoreCase(Pageable pageable, BaseRequestStatusType baseRequestStatusType, String initiator);

    Page<AccountRequest> findByBaseRequestInitiatorIgnoreCase(Pageable pageable, String loggedUserName);

}

Now I was wondering if there would be any way to refactor this code so that I could get rid of the duplicate. I tried to find a solution using Java 8 and passing around functions but I can't really figure how to parametrize this.

Upvotes: 1

Views: 400

Answers (2)

Hulk
Hulk

Reputation: 6583

If you cannot or for some reason don't want to modify your existing class structure, there are still ways to refactor such code. You could, for example, pass your query-functions as parameters to a generic function that decides which of the queries to invoke, for example

public Page<AccountRequest> getAccountRequestPageable(final Pageable pageable, final String status) {
    return getPageable(pageable, status,
            p -> accountRequestRepository.findByBaseRequestInitiatorIgnoreCaseAndBaseRequestOutdatedDaysGreaterThanAndBaseRequestStatus(p, LoggedUserUtil.getLoggedUserName(), 0, BaseRequestStatusType.APPROVED),
            p -> accountRequestRepository.findByBaseRequestInitiatorIgnoreCase(p, LoggedUserUtil.getLoggedUserName()),
            p -> accountRequestRepository.findByBaseRequestStatusAndBaseRequestInitiatorIgnoreCase(p, BaseRequestStatusType.valueOf(status), LoggedUserUtil.getLoggedUserName()));
}

public Page<ApplicationsRequest> getApplicationsRequestPageable(final Pageable pageable, final String status) {
    return getPageable(pageable, status, 
            p -> applicationRequestRepository.findByBaseRequestInitiatorIgnoreCaseAndBaseRequestOutdatedDaysGreaterThanAndBaseRequestStatus(pageable, LoggedUserUtil.getLoggedUserName(), 0,BaseRequestStatusType.APPROVED),                
            p -> applicationRequestRepository.findByBaseRequestInitiatorIgnoreCase(pageable, LoggedUserUtil.getLoggedUserName()),
            p -> applicationRequestRepository.findByBaseRequestStatusAndBaseRequestInitiatorIgnoreCase(pageable, BaseRequestStatusType.valueOf(status), LoggedUserUtil.getLoggedUserName()));                            
}
           
public static <T,Q> Page<Q> getPageable(final Pageable pageable, final String status, 
        Function<Pageable, Page<Q>> findUncompleted,
        Function<Pageable, Page<Q>> findAll,
        Function<Pageable, Page<Q>> findOther
        ) {

    Page<Q> accountRequestPage;

    if (UNCOMPLETED_STATUS.equals(status)) {
        accountRequestPage = findUncompleted.apply(pageable);
    } else {
        if (!ALL_STATUS.equals(status)) {
            accountRequestPage = findOther.apply(pageable);
        } else {
            accountRequestPage = findAll.apply(pageable);
        }
    }
    return accountRequestPage;
}

Whether or not such a refactoring makes sense depends on the project, and cannot be answered in general. When thinking about code duplication, it is important to consider the possibility that the similarity may just be coincidence. If two segments code look similar now, but will need to change independently in the future, it may be better to keep them separate. If they really share an underlying principle in your business domain that can be abstracted out, do so. In other words, follow the Single Responsibility Principle, in the sense of "single reason to change".

Upvotes: 1

Jonathan Schoreels
Jonathan Schoreels

Reputation: 1720

I think what you need is to create "Request" interface and use the Entity Inheritance of Hibernate (which will be understood by Spring Data) to get Page<? extends Request>. See https://www.baeldung.com/hibernate-inheritance.

Then, you can put the common logic using the Request interface, or even define Request as a super class where you'll store that common logic, your call !

Upvotes: 1

Related Questions