[REFACTOR] 백엔드 레거시 코드 리펙터링

[kyum-q]

📌 어떤 기능을 리팩터링 하나요?

리팩터링 할 기능에 대해 간결하게 설명해주세요

리펙터링 하고 싶은 내용을 이슈 코멘트로 작성하고 하나씩 해결해봐요 !

⏳ 예상 소요 시간 (예상 해결 날짜)

10일

🔍 참고할만한 자료(선택)

[kyum-q]

@Disabled 된 테스트 검토

TagService

템플릿으로 태그 조회

• 기능: template을 인자로 받아 해당 템플릿의 모든 태그를 조회.
• 문제: 존재하지 않는 템플릿에 대한 태그 조회 시 별도의 예외 처리가 없다.
• 상황: 이 메서드는 파사드 서비스에서 호출되며, 템플릿의 존재 여부는 파사드 서비스에서 판단하므로 내부에서 예외 처리를 하지 않음.
• 해결 방안:
  1. 내부에서도 템플릿 존재 여부를 확인한다.
  2. 현재 방식 유지: 외부 로직을 신뢰하고 내부에서 추가 체크를 하지 않는다.
• 경미 생각: 현재 방식을 유지.
  • 내부에서 체크를 추가하면 데이터베이스 접근이 증가해 성능에 영향을 줄 수 있다.
  • 이 메서드는 template이라는 도메인을 받아 처리하는 특성상 외부 로직의 유효성을 신뢰하는 것이 적절하다.

SourceCodeService

소스코드 업데이트 및 생성 시 소스코드 개수 검증

• 기능: 소스코드 서비스의 생성 및 업데이트 시 소스코드 개수 확인 및 예외 처리
• 문제: (업데이트 시) 소스코드가 0개일 시, 애플리케이션 코드 상 예외 발생이 안된다. 하지만 썸네일을 설정하는 코드에서 예외가 발생한다. (thumbnail으로 인해 DataIntegrityViolationException 발생)
• 상황: 이 메서드는 파사드 서비스에서 호출되며, 템플릿의 존재 여부는 요청이 들어온 후 @Valid로 인해 예외 처리가 되고 있어서, 내부적으로 예외처리하지 않음을 인지하지 못했음.
• 경미 생각: 내부에서 한 번 더 체크하는 방식으로 수정한다.
  • 데이터베이스에 접근하는 복잡한 로직이 아님. 중복 처리한다고 문제될거나 성능 저하되지 않는다.
  • 방어적 프로그래밍은 유지보수성을 높이고, 잘못된 사용으로 인해 발생할 수 있는 버그를 예방하면 좋을 것 같다.

소스코드 업데이트 및 생성 시 소스코드 순서 검증

• 기능: 소스코드 서비스의 생성 및 업데이트 시 소스코드 순서 확인 및 관한 예외 처리
• 경미 생각: 소스코드 개수 검증과 동일한 이유로 내부에서 한 번 더 체크하는 방식으로 수정한다.

추가 수정 사항
소스 코드 순서 검증 시, 이전에는 순서가 정렬되지 않아도 예외를 발생시켰다. 이를 수정하여 정렬되지 않아도 순서가 모두 존재한다면 예외처리하지 않는다.
ex) 1,2,4,3 - 이전: 예외 발생 O -> 이후 예외 발생 X

일부 소스 코드 삭제 및 새로운 소스 코드 추가 시, 삭제된 코드 순서는 앞당겨지고 새로 추가된 소스 코드의 순서는 가장 마지막 순서

• 경미 생각 : 소스코드를 앞당기는 코드가 있어야하나 싶다..! 현재 프론트에서 존재하는 코드를 기준으로 순서를 주기 때문에 혼란이 있을 것 같음. 이경우는 예외처리되는게 맞다고 생각이 듭니다.
  • ex) 순서1, 순서2 -> (순서3 추가, 순서2 삭제) -> 순서3가 자연스럽게 순서2가 되어야한다는 것
  • 하지만 나는 이게 부자연스럽다고 생각

TemplateService

Pageable에 대한 null 검증

• 기능: Pageable을 인자로 받아 페이지 처리해주는 기능
• 문제: 내부에서 Pageable에 대한 null 검증이 없어서 null을 입력하면 예외가 발생한다.
• 상황: 템플릿의 존재 여부는 컨틀롤러에서 판단하므로 내부에서 예외 처리를 하지 않음.
• 해결 방안:
  1. 내부에서도 Pageable null 검증한다.
  2. 현재 방식 유지: 외부 로직을 신뢰하고 내부에서 추가 체크를 하지 않는다.
• 경미 생각: 현재 방식을 유지.
  • 현재 컨트롤러 단에서 @PageableDefault를 사용해서 null이 될 가능성이 없다고 판단
  • 이렇게 내부 검증을 원할 경우 사용하는 가장 최하단에서 체크해야한다고 생각한다. 하지만 내부에 Pageable을 사용하는 메서드가 너무 많다. 그래서 오히려 코드 중복이 생기고 모든 메서드에서 일일이 null 체크를 하는 것은 비효율적이라고 생각합니다.

하지만 적절한 중앙 검증 위치가 있다면 추가하면 좋을 듯하다. 하지만 현재는 컨트롤러에서 처리해주는 @PageableDefault로 적절한 중앙 검증이 된다고 생각합니다.