ZinnaChoi / Study-Management

온라인 스터디 관리 시스템: 부재 일정 캘린더, 토론 게시판 및 알림 기능 제공
8 stars 0 forks source link

Task add create general notice #64

Closed dayeon-dayeon closed 7 months ago

dayeon-dayeon commented 7 months ago

안녕하세요!

  1. 구글 링크 생성 알림 API 입니다.

  2. 카카오톡 알림 대신 이메일 전송으로 구현하였습니다. image

  3. 디렉토리명을 batch에서 scheduler로 변경하였습니다.

  4. statController에서 finally 구문에서 endAPI()를 호출하지 않는 부분 추가하였습니다.

NoticeControllerTest.java 에서 테스트 가능합니다 !


부재일정 등록, 새 글 등록, 기상 성공 여부 체크 후 알림도 추가했습니다.

MeMyself-And-I commented 7 months ago

@dayeon-dayeon 고생 많으셨습니다. 새로 구현된 부재일정 등록, 새 글 등록, 기상 성공 여부 체크 후 알림도 관련 컨트롤러 테스트 코드에서 돌려보니 정상적으로 동작하네요!

하기 내용은 리뷰 하면서 느낀점이고 굳이 수정해야 할 사항은 아니지만 레이어 관리, 의존성 분리 측면에서 괜찮을 것 같아 제 개인적인 생각을 남겨봅니다!

서비스 레이어에서 알람을 발생해야하는 여러개의 서비스가 있고(AbsentServiceImpl, PostServiceImpl, StatServiceImpl 등) 해당 서비스 레이어는 NoticeServiceImpl을 의존성 주입 받아 객체 내의 메소드를 사용하고 있습니다(createSpecificNotice()), 또 NoticeServiceImpl 서비스는 SendEmailService의 의존성을 주입받고 있습니다. 이런 형식을 보면 알람 발생 서비스들(AbsentServiceImpl, PostServiceImpl, StatServiceImpl 등)은 NoticeServiceImpl의 공통적인 메소드를 사용하고 있으니, 자바에서는 다중 상속은 지원하지 않지만 다중 인터페이스 구현은 가능하니 또 NoticeServiceImpl를 인터페이스화 하여 AbsentServiceImpl, PostServiceImpl, StatServiceImpl와 같은 서비스가 구현(Override)하도록 하거나, 로직이 전부 동일하다면 인터페이스의 default 메소드를 사용하여 동일한 로직의 메소드를 사용할 수 있도록 하는 등의 조금 더 추상화 할 수 있는 방법들이 있을 것 같습니다.

간단하게 요약하자면,

  1. SendEmailService과 NoticeService의 병합 및 인터페이스화 or 추상 클래스 화
  2. 알람을 발생해야하는 여러개의 서비스들이(AbsentServiceImpl, PostServiceImpl, StatServiceImpl 등) 이를 구현

을 통해서 조금 더 java, spring boot가 지향하는 설계 원칙을 지킬 수 있을지 않을까 생각합니다!

제가 틀린 부분이 있을 수 있고, 실제로 저 방법대로 했을 때 가독성 면에서 더 불편할 수도 있으나 코드 리뷰를 하면서 제 생각을 조금 남겨봅니다.. 수정 사항은 아닙니다!!