How to avoid duplicate code with a helper-method for shared REST-operations like GET and DELETE-CodePudding

I have two methods

methodOne (e.g. for a GET on questionaire)
methodTwo (e.g. for a DELETE on questionaire)

Both have almost same lines of code.

IntelliJ also underlines the code and shows a warning:

duplicated code fragment

`methodOne`

@Override
public QuestionnaireDTO methodOne(long projectManagerId, long questionnaireId) {

    //the part I want to write into a helper method
    ProjectManager projectManager = projectManagerRepository.findById(projectManagerId).orElseThrow(
            () -> new ResourceNotFoundException("ProjectManager", "id", projectManagerId));

    Questionnaire questionnaire = questionnaireRepository.findById(questionnaireId).orElseThrow(() ->
            new ResourceNotFoundException("Questionnaire", "id", questionnaireId));

    if(!questionnaire.getProjectManager().getId().equals(projectManager.getId())){
        throw new QuestionnaireApiException(HttpStatus.BAD_REQUEST, "Questionnaire not belonging to Project Manager");
    }
    // end of helper method

    // return of methodOne 
    return mapToDto(questionnaire);
    
}

`methodTwo`

@Override
public QuestionnaireDTO methodTwo(long projectManagerId, long questionnaireId) {

    //the part I want to write into a helper method
    ProjectManager projectManager = projectManagerRepository.findById(projectManagerId).orElseThrow(
            () -> new ResourceNotFoundException("ProjectManager", "id", projectManagerId));

    Questionnaire questionnaire = questionnaireRepository.findById(questionnaireId).orElseThrow(() ->
            new ResourceNotFoundException("Questionnaire", "id", questionnaireId));

    if(!questionnaire.getProjectManager().getId().equals(projectManager.getId())){
        throw new QuestionnaireApiException(HttpStatus.BAD_REQUEST, "Questionnaire not belonging to Project Manager");
    }
    // end of helper method

    // return of methodTwo 
    return questionnaireRepository.delete(questionnaire);
    
}

Questions

I want to write the duplicated part of the code (enclosed in comments above) into a helper method. I want to avoid duplicate code but I'm not sure how to structure the helper method.

How can I achieve it?

CodePudding user response：

You can actually just move the code 1:1 to a new method:

public QuestionnaireDTO methodTwo(long projectManagerId, long questionnaireId) {
    return mapToDto(findById(projectManagerId, questionnaireId));
}

public QuestionnaireDTO methodOne(long projectManagerId, long questionnaireId) {
    return questionnaireRepository.delete(findById(projectManagerId, questionnaireId));
}

private QuestionnaireDTO findById(long projectManagerId, long questionnaireId) {
    ProjectManager projectManager = projectManagerRepository.findById(projectManagerId).orElseThrow(
            () -> new ResourceNotFoundException("ProjectManager", "id", projectManagerId));

    Questionnaire questionnaire = questionnaireRepository.findById(questionnaireId).orElseThrow(() ->
            new ResourceNotFoundException("Questionnaire", "id", questionnaireId));

    if(!questionnaire.getProjectManager().getId().equals(projectManager.getId())){
        throw new QuestionnaireApiException(HttpStatus.BAD_REQUEST, "Questionnaire not belonging to Project Manager");
    }

    return questionnaire;
}

CodePudding user response：

The shared code can return a Questionnaire object or throw a ResourceNotFoundException:

private Questionnaire findQuestionnaire(long projectManagerId, long questionnaireId) {
    ProjectManager projectManager = projectManagerRepository.findById(projectManagerId).orElseThrow(
            () -> new ResourceNotFoundException("ProjectManager", "id", projectManagerId));

    Questionnaire questionnaire = questionnaireRepository.findById(questionnaireId).orElseThrow(() ->
            new ResourceNotFoundException("Questionnaire", "id", questionnaireId));

    if (!questionnaire.getProjectManager().getId().equals(projectManager.getId())){
        throw new QuestionnaireApiException(HttpStatus.BAD_REQUEST, "Questionnaire not belonging to Project Manager");
    }
    
    // return questionnaire if both IDs are valid
    return questionnaire;
}

CodePudding user response：

TL;DR: When IntelliJ warns about "duplicated code fragment" then Extract Method refactoring is a solving IDE-shortcut.

Always pays-off to add more context like used IDE and shown error-message to the question.

Before posted answers (from knittl and mureinik) show you the resulting code: definition of extracted helper-method findById and its usage in given methods methodOne and methodTwo.

This answer shows how IntelliJ supports you to follow the DRY principle and avoid duplication (most other IDEs have similar functions).

Extract Method refactoring

A common refactoring method, to either break code up and improve readability or to avoid duplication and follow DRY is: Extract Function or Extract Method

IntelliJ IDEA has an action for Extract Method:

The Extract Method refactoring lets you take a code fragment that can be grouped, move it into a separated method, and replace the old code with a call to the method.

To use the refactoring technique Extract Method select the code-block you want to extract (e.g. code enclosed in your given comments). Then you can choose among 4 ways as you prefer:

hover over the code (sometimes bulb-symbol appears) and press Alt Enter to show Intention Actions
right-click to open the context-menu, then navigate to: refactor, extract-method
Find action (Win/Linux: Ctrl Shift A) by name "Extract Method"
press the keyboard-shortcut for Extract Method action Ctrl Alt M (Win/Linux, different on Mac)

Intention Actions help on errors in IntelliJ

Since this error-message is shown as a result of IntelliJ's Code Inspection, it might be prepared to help you solving it.

In many cases (like here) these 1-2 steps are the quickest way from error to fix:

Jump to issues/errors in code pressing F2 (Win/Linux). Those issues/errors can also be found by yellow gutter-marker or a red gutter-marker and bulb-symbol showing up.
Use Intention Actions: Click on the bulb-symbol or press Alt Enter (Win/Linux) to show IntelliJ's Intention Actions. Select "Extract method from duplicate code"

See also: IntelliJ IDEA Documentation: Analyze duplicates to learn about IDE-support in finding and fixing duplication.

When and how extract-method in IntelliJ works

First a modal-dialog opens where you can enter the name of the method. Usually IntelliJ suggests the name and parameters that are used as input.

More important is the return-value or output. A method or function can only be extracted if either no or exactly one single return-value or output can be determined. In you case Questionnaire questionnaire is the singular output.

After confirming the dialog, your extracted helper-method was created by IntelliJ and all previous locations from where the code was extracted are replaced by a respective method-call.