CODEWITHSUNDEEP
javarefactoring
fojalar
fojalar

Reputation: 47

Consolidating two methods into one

I recently went through a code review, and it was firmly suggested that I consolidate two methods into one. Both methods are identical, save for a single method call in each, and one does not require an argument.

Method #1

private void updateCache(List<CategoryObject> objectList) {
    ServiceApi serviceApi = getService();
    if (serviceApi != null) {
        try {
            serviceApi.updateResources(objectList);
        } catch (BusinessException e) {
            log.error(e);
        }
    }
}

Method #2

private void registerCache() {
    ServiceApi serviceApi = getService();
    if (serviceApi != null) {
        try {
            serviceApi.registerCategory(CATEGORY_NAME);
        } catch (BusinessException e) {
            log.error(e);
        }
    }
}

Can these even be efficiently combined?

Upvotes: 0

Views: 249

Answers (2)

OldCurmudgeon
OldCurmudgeon

Reputation: 65889

You can pull the inner functionality out into an interface:

private interface Op {
    void perform(ServiceApi serviceApi);
}

static void cache(Op op) {
    ServiceApi serviceApi = getService();
    if (serviceApi != null) {
        try {
            op.perform(serviceApi);
        } catch (BusinessException e) {
            log.error(e);
        }
    }

}

private void updateCache(List<CategoryObject> objectList) {
    cache(new Op() {
        @Override
        public void perform(ServiceApi serviceApi) {
            serviceApi.updateResources(objectList);
        }
    });
}

private void registerCache() {
    cache(new Op() {
        @Override
        public void perform(ServiceApi serviceApi) {
            serviceApi.registerCategory(CATEGORY_NAME);
        }
    });
}

In Java 8 the two methods become truly simple and elegant.

private void updateCache(List<CategoryObject> objectList) {
    cache(serviceApi -> serviceApi.updateResources(objectList));
}

private void registerCache() {
    cache(serviceApi -> serviceApi.registerCategory(CATEGORY_NAME));
}

Upvotes: 3

hamena314
hamena314

Reputation: 3109

You could just use one method and differentiate by the input parameter:

private void updateCache(List<CategoryObject> objectList) {
    ServiceApi serviceApi = getService();
    if (serviceApi != null) {
        try {
            if(objectList != null){
                serviceApi.updateResources(objectList);                
            }
            else{
                serviceApi.registerCategory(CATEGORY_NAME);
            }
        } catch (BusinessException e) {
            log.error(e);
        }
    }
}

Maybe the method name should be refactored as well then: handleCache()?

You then can call the method in 2 ways:

handleCache(objectList) --> works like method #1

handleCache(null) --> works like method #2

Upvotes: 0

Related Questions