Refactoring with SonarLint - Service

SonarLint라는 툴이 권장하는 방향으로 코드 리팩토링을 하면서 리팩토링 규칙들을 기록하는 글입니다! 중복되는 규칙들은 한 번만 작성하였습니다.

💡 BuildingServiceImpl

• 사용하지 않는 메소드 파라미터는 삭제하라
  • 해당 메소드 자체를 사용하지 않아서 삭제했습니다.

// Noncompliant Code
public Building updateBuilding(BuildingOptionalDto buildingOptionalDto) {
      return null;
}

 

💡 FileProcessServiceImpl

• replaceAll 함수를 호출하는 대신 replace 함수를 호출하라
  • replace 함수로 대체했습니다.
  • 설명
    • String::replaceAll 메소드는 호출될 때마다 java.util.regex.Pattern.compile() 메소드를 호출합니다. 이는 자원을 상당히 많이 소모하므로 신중하게 사용해야 합니다.
    • 특히, String::replaceAll 의 첫 번째 인수가 정규표현식이 아닌 경우에 이러한 성능 저하가 더욱 두드러집니다.
    • 따라서, 첫 번째 인수가 실제 정규표현식이 아닌 경우 String::replaceAll 대신 String::replace를 사용하는 것이 좋습니다.
    • String::replace는 String::replaceAll와 같은 역할을 하지만, 정규 표현식을 처리하는 데 드는 부담을 갖지 않습니다.

// Noncompliant Code
protected String createValidOriginFileName(String originalFileName) {
    String trimedString = originalFileName.trim();
    String result = trimedString.replaceAll(" ", "-");
        return result;
}

// Compliant Solution
protected String createValidOriginFileName(String originalFileName) {
    String trimedString = originalFileName.trim();
    String result = trimedString.replaceAll(" ", "-");
        return result;
}

• 생성자의 접근 제한자를 protected로 변경해라
  • 해당 클래스가 추상 클래스여서 권장된 규칙입니다. protected로 변경했습니다.
  • 설명
    • 추상 클래스는 그 자체로는 인스턴스화할 수 없으며, 그것의 하위 클래스에 의해서만 인스턴스화될 수 있습니다.
    • 이러한 이유로, 추상 클래스의 생성자는 그 하위 클래스의 생성자에서만 호출될 수 있습니다.
    • 따라서, 추상 클래스의 생성자를 public으로 선언하는 것은 불필요합니다. 외부에서 접근할 수 없기 때문입니다.
    • 대신, protected 접근 제한자를 사용하여 하위 클래스에서만 접근할 수 있게 하면 충분합니다.

// Noncompliant Code
public FileProcessServiceImpl(FileService amazonS3Service, UuidCustomRepositoryImpl uuidCustomRepository, UuidRepository uuidRepository) {
    this.amazonS3Service = amazonS3Service;
    this.uuidCustomRepository = uuidCustomRepository;
    this.uuidRepository = uuidRepository;
}

// Compliant Solution
protected FileProcessServiceImpl(FileService amazonS3Service, UuidCustomRepositoryImpl uuidCustomRepository, UuidRepository uuidRepository) {
    this.amazonS3Service = amazonS3Service;
    this.uuidCustomRepository = uuidCustomRepository;
    this.uuidRepository = uuidRepository;
}

• 임시 변수에 할당하여 리턴하는 대신 즉시 리턴해라
  • filePath라는 임시변수를 두어서 리턴하는 대신 즉시 리턴하도록 변경했습니다.

// Noncompliant Code
public String getFilePath(MultipartFile file, T imagePackage) {
    AmazonS3PackageCommand command = imagePackage.createCommand();
    String uuid = imagePackage.getUuid();
    String filePath = amazonS3Service.getFileFolder(command) + createFileName(uuid, file.getOriginalFilename());
    return filePath;
}

// Compliant Solution
public String getFilePath(MultipartFile file, T imagePackage) {
    AmazonS3PackageCommand command = imagePackage.createCommand();
    String uuid = imagePackage.getUuid();
    return amazonS3Service.getFileFolder(command) + createFileName(uuid, file.getOriginalFilename());
}

• 원시 boolean 표현식을 작성해라
  • 아래 코드에서 exist함수는 박싱된 타입인 Boolean을 리턴하는데 박싱된 타입은 nullable하므로 기존 코드에서는 NullPointerException이 발생할 수 있습니다.
  • 코드를 null safe하게 변경했습니다.

// Noncompliant Code
public Uuid createUUID() {
    Uuid savedUuid = null;
    String candidate = UUID.randomUUID().toString();
    if (uuidCustomRepository.exist(candidate)) {
        savedUuid = createUUID();
    }
    savedUuid = uuidRepository.save(Uuid.builder().uuid(candidate).build());
    return savedUuid;
}

// Compliant Solution
public Uuid createUUID() {
    Uuid savedUuid = null;
    String candidate = UUID.randomUUID().toString();
    Boolean doesExist = uuidCustomRepository.exist(candidate);
    if (Boolean.TRUE.equals(doesExist)) {
        savedUuid = createUUID();
    }
    savedUuid = uuidRepository.save(Uuid.builder().uuid(candidate).build());
    return savedUuid;
}

 

💡 ProfileImageServiceImpl

• Math.random 함수 대신에 java.util.Random.nextInt 함수를 사용해라
  • Math.random은 내부적으로 Random.nextDouble를 호출하며, 이는 Random.nextInt에 비해 더 많은 연산을 수행해야 합니다.
  • 또한 아래 기능에서는 int로 형변환까지 해줘야하므로 추가적인 비용이 듭니다.
  • 따라서, nextInt 함수를 사용해 리팩토링해주었습니다.

// Noncompliant Code
@Override
public ProfileImage random() {
    Long totalCount = profileImageRepository.count();
    int randomIndex = (int) (Math.random() * totalCount);
    Page<ProfileImage> profileImageUrlPage = profileImageRepository.findAll(PageRequest.of(randomIndex, 1));
    return profileImageUrlPage.getContent().get(0);
}

// Compliant Solution
private Random random = new Random();

@Override
public ProfileImage random() {
    Long totalCount = profileImageRepository.count();
    int randomIndex = random.nextInt(totalCount.intValue());
    Page<ProfileImage> profileImageUrlPage = profileImageRepository.findAll(PageRequest.of(randomIndex, 1));
    return profileImageUrlPage.getContent().get(0);
}

 

💡 ReviewLikeServiceImpl

• 값에 접근하기 전에 Optional의 isPresent 혹은 isEmpty를 호출해라.
  • isPresent, isEmpty를 쓰는 대신, 예외를 던지는 방법으로 리팩토링했습니다.

// Noncompliant Code
Review review = reviewRepository.findById(reviewId).get();

// Compliant Solution
Review review = reviewRepository.findById(reviewId).orElseThrow(() -> new ReviewException(ErrorCode.REVIEW_NOT_FOUND));