[feat] 문제 수정, 삭제 API 구현 #44
Conversation
| questionRepository.delete(question); | ||
| } | ||
|
|
||
| private void validateAnswer(String answer) { |
There was a problem hiding this comment.
validate 분리되어있는거 편--안하네요 👍
| textQuestion.changeContent(request.content()); | ||
| } | ||
|
|
||
| if (request.answer() != null) { |
There was a problem hiding this comment.
[L4-변경제안]
content, answer에 대한 null 체크를 컨트롤러에서 검증하면 서비스 코드 구조가 조금 더 단순해질 것 같습니다 !
There was a problem hiding this comment.
옹.. 잘 이해가 안되는데, 컨트롤러에서 null 체크를 하고, QuestionService에 문제를 바꾸는 메서드, 정답을 바꾸는 메서드 이렇게 따로 만들어 사용하라는 말씀이신가요?
There was a problem hiding this comment.
넵 ! content, answer의 null 체크는 컨트롤러에서 수행하고 QuestionService의 메서드를 호출하면 if문이 없어도 되지 않을까 생각했습니다 ! 비즈니스 로직과 관련이 있지 않다면 컨트롤러에서 검증해도 된다고 생각합니다 !
There was a problem hiding this comment.
아!! 이해했습니다. 확실히 구분하니까 content 수정할 때만 TextQuestion을 불러오면 되니 불필요한 JOIN도 줄고, 역할 분리가 제대로 되는 것 같습니다. 좋은 의견 감사합니다 !!
There was a problem hiding this comment.
근데 궁금한 게 있는데, 지금처럼 분리하면 question을 불러오기 위해 findById()를 두 메서드에서 두 번이나 사용하게 되는데 (조회 쿼리 두번), 한 메서드에서 한 번 불러오고 (조회 쿼리 한 번) null 검사를 해주는 것보다 SQL이 많이 날라가게 되는 건 신경쓰지 않아도 될까요?
There was a problem hiding this comment.
아 저는 컨트롤러와 서비스의 역할이 분리가 되면 좋겠다고 생각해서 말씀드린건데 DB 조회 관련해서는 생각하지 못했네요,,, pk인 ID에 대한 인덱스가 있기 때문에 특정 ID를 조회하는 쿼리는 그렇게 성능 저하가 없을 것으로 예상되긴 합니다 ! 제 리뷰의 요지는 컨트롤러는 요청을 서비스로 분기하는 역할을 가지니까 컨트롤러에서 검증할 수 있는 것은 검증해서 서비스의 특정 비즈니스 로직을 수행하도록 하면 역할 분리가 더 잘될 것 같다는 취지에서 말씀드렸습니다 !
There was a problem hiding this comment.
옹 그렇군요 ! 저도 역할 분리가 잘 될 것 같다는 것은 완전 동의합니다 !! 그래서 QuizService에서도 updateQuiz 메서드 분리하는데,, 문득 궁금해져서 여쭤봤습니다 ㅎㅎ.. 알려주셔서 감사합니다 !! :) 👍
| quiz.changeTitle(request.title()); | ||
| } | ||
|
|
||
| if (request.description() != null) { |
There was a problem hiding this comment.
[L4-변경제안]
이 부분도 위의 리뷰와 같은 내용입니다 !
|
|
||
| @Override | ||
| public boolean isValid(String value, ConstraintValidatorContext context) { | ||
| if (value == null) return true; |
There was a problem hiding this comment.
[L5-참고의견]
value가 null일 경우 true return이 정상 흐름인지 확인해 주시면 감사하겠습니다.
There was a problem hiding this comment.
넵 맞습니다 ! null 검사는 @NotNull 이 따로 있기 때문에 @TrimmedSize 어노테이션은 공백 제외 글자수를 확인해주는 용도로 사용하기 위해 분리해두었습니다!
현재 유효성 검사를 할 때, @TrimmedSize와 @NotBlank를 해주고 있는데,, 유효성 검사를 처리하는 순서가 @NotBlank -> @TrimmedSize와 같은 커스텀 어노테이션 이렇게 된다고 합니다!
그래서 완전한 빈 문자열이나 null이 들어왔을 땐 @NotBlank에 걸려서 -> 제목을 설정해주세요.
그 이후 문자열이 있긴 있는데, 글자 수가 충족이 되지 않을 땐 @TrimmedSize -> 몇 자 이상 ~ 으로 설정해주세요.
이렇게 역할이 분리되는 것이 맞다고 생각하여 따로 null 처리는 하지 않았습니다 !
There was a problem hiding this comment.
TrimmedSize라는 어노테이션이 개발자의 실수 등의 이유로 단독으로 사용되는 경우를 생각했었는데,
말씀하신 것처럼 Null 처리에 대한 역할은 NotNull NotBlank에 위임하고 TrimmedSize는 글자 수 검증 용도로만 사용하는 것이 맞는 것 같습니다.
감사합니다!
|
|
||
| import java.lang.annotation.*; | ||
|
|
||
| @Documented |
There was a problem hiding this comment.
[L5-참고의견]
@TrimmedSized 어노테이션을 닉네임에도 적용하면 좋을 것 같네요!
배우고 갑니다!
🛰️ Issue Number
🪐 작업 내용
1. Question 수정, 삭제 API 구현
문제 수정, 삭제 API를 구현했습니다. 포스트맨으로 정상 동작하는 것 확인했습니다.
{ "content": "새 문제", "answer": null }question->content로 변경했습니다.2. 길이 제한 설정
수정할 때 빈 문자열을 보내올 수도 있겠다 싶어 퀴즈 제목, 설명 / 퀴즈 문제, 정답도 글자 수에 제한을 둬야겠다고 생각했습니다.
QuizCreateRequest와QuestionCreateRequest에서@NotBlank와@Size로 유효성 검사를 진행합니다.근데, 이랬을 경우 수정 때는 변경되지 않은 값은
null로 들어올 수 있기 때문에DTO차원에서 유효성 검사를 해줄 순 없어 서비스에서 글자 수를 검사합니다." "이러한 문자열이 들어올 수 있다고 생각했습니다.@NotBlank검사를 해줄 수 없기 때문에 서비스에서trim()을 해줘야 하는데 그럼 글자 수 비교가 생성할 때와 수정할 때 기준이 좀 달라지게 됩니다. (공백 포함 vs 공백 제외)3. TrimmedSize 어노테이션 추가
그래서
@TrimmedSize라는 어노테이션을 추가했고,ConstraintValidator를 구현하여 Validator를 정의했습니다.이에 따라 퀴즈의 제목, 설명 / 퀴즈의 문제, 정답 글자 수 제한은 다음과 같습니다.
📚 Reference
✅ Check List