[시온] step6. 페이지 나누기 #105
[시온] step6. 페이지 나누기 #105
Conversation
| return null; | ||
| } | ||
| User sessionedUser = HttpSessionUtils.getUserFromSession(session); | ||
| answer.setWriter(sessionedUser); |
There was a problem hiding this comment.
이것까지 AnswerService 에서 해 주면 어떨까요? .create() 에서 User 도 파라메터로 받고요.
There was a problem hiding this comment.
그렇게 하는게 역할을 더 명확하고 분리하고 일관성 있게 해줄 것 같습니다 ㅎㅎㅎ
수정하겠습니다~!
| if (!answer.isWrittenBy(loginUser)) { | ||
| throw new IllegalStateException("자신이 작성한 답변만 삭제할 수 있습니다."); | ||
| } |
There was a problem hiding this comment.
이 예외도 AnswerService 안에서 던져질 수 있으면 더 좋을 것 같습니다.
There was a problem hiding this comment.
예외를 Controller에서도 던지고 Service에서도 던져서 중구난방인 것 같은 느낌이 들었는데 좋은 것 같습니다!
그런데 브라이언의 리뷰를 받고, 그렇다면 Controller에서 던지는게 맞을까, Service에서 던지는게 맞을까 라는 고민이 문득 들었습니다.
Service에서 던질 수 있으면 그렇게 하는 것이 좋은 건지, 아니면 꼭 정해진 건 없고 상황에 맞게 던지는게 맞는 건지 잘 모르겠어서 추가 질문 드립니다!
| } | ||
|
|
||
| @GetMapping("/") | ||
| public Page<Question> pagingList(Criteria cri) { |
There was a problem hiding this comment.
cri 는 좋은 축약 표현이 아닌 것 같아요. criteria 라고 끝까지 써주세요.
There was a problem hiding this comment.
cri라는게 뭐하는 건지 한눈에 안들어오네요 ㅎㅎㅎ ㅠㅠ
수정하겠습니다~!
| public String goMain(Model model) { | ||
| model.addAttribute("questions", questionService.findAll()); | ||
| public String goMain(Optional<Integer> pageNum, Model model) { // 메인화면(Url 매핑 -> "/") 진입 시 1페이지로 설정 | ||
| Criteria cri = new Criteria(pageNum.orElse(new Integer(1))); |
There was a problem hiding this comment.
Integer, String 과 같은 타입들의 생성자를 호출해서 사용하는 것과, 그냥 원시 타입과 같게 호출하는 것과 어떤 차이가 있는데요, 어떤 차이일까요?
앞으로 Integer, Long 과 같은 래퍼 타입을 다룰 때 꼭 알아야하는 내용이라, 제가 설명하기보다는 시온이 직접 찾아보시고 그 차이점에 대해 꼭 숙지하셨으면 좋겠어요.
There was a problem hiding this comment.
위 코드에서는 pageNum을 Integer로 타입을 지정해주었기 때문에 orElse에 new Integer(1)을 매개변수로 주었는데요, 원시 타입으로 1을 주었어도 오토박싱으로 Integer타입이 될 것 같습니다 ㅎㅎㅎ
저는 오토박싱과 언박싱에 대한 부분만 알 것 같은데요.. ㅠㅠ 혹시 브라이언이 질문하신 의도가 오토박싱과 언박싱이 맞는지, 아니라면 힌트좀 더 주실 수 있을까요!?
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class PageDTO { |
There was a problem hiding this comment.
전반적으로 주석이 좀 장황한 느낌인데요,
적절한 주석도 있지만 그렇지 않은 주석도 있어요. 이를테면 startPage 가 목록의 시작 페이지 인 것은 누가 봐도 알 수 있겠군요(그만큼 시온이 변수명을 잘 지었다는 뜻도 되어요).
There was a problem hiding this comment.
주석을 써주면 성의있어 보이고 좋은 것 같은데 과할 경우도 있겠군요 ㅎㅎㅎ 쓰는 것도 일이었는데 적절하게 사용하고 변수명으로 의도를 더 명확하게 전달할 수 있도록 고민해봐야겠습니다~!
| private int startPage; // 페이지 목록의 시작 페이지 | ||
| private int endPage; // 페이지 목록의 마지막 페이지 | ||
| private boolean prev; // 페이지 목록의 "<<" 버튼 활성화 여부 | ||
| private boolean next; // 페이지 목록의 ">>" 버튼 활성화 여부 | ||
| private int prevEndPage; // "<<" 버튼 클릭시 startPage 이전 페이지로 이동, [ << 6 7 8 ] 일 경우 5로 이동 | ||
| private int nextStartPage; // ">>" 버튼 클릭시 endPage 다음 페이지로 이동, [ 1 2 3 4 5 >> ] 일 경우 6으로 이동 | ||
|
|
||
| private int total; // 화면에 출력할 수 있는(삭제되지 않은) 게시물의 총 개수 |
There was a problem hiding this comment.
Spring Framework가 제공하는 Page 객체의 Javadoc 을 참고해주세요.
그리고, 이 필드들 중 Page 객체가 대체할 수 없는 정보가 얼마나 있는지도 파악해주세요.
There was a problem hiding this comment.
이 필드 중에 Page의 메서드로 곧바로 대체할 수 있는 필드는 total -> getTotalElements() 인 것 같습니다.
다른 필드들은 제가 구현한 방법에선 연산이 필요하고, 곧바로 대체 적용할 수 있는 메서드가 없는 것 같아서 고민을 해보았는데요, Page를 매개변수로 받고 pageable에 있는 페이지번호, 사이즈 정보를 통해서 필드 없이 getter에서 곧바로 연산해서 return해주는 방식으로 위의 필드들은 모두 없앴습니다.
제가 생각하기엔 생성자에서 각 필드의 값을 연산하여 저장한 것을 단지 Page를 매개변수로 받아서 사용한 것과 연산을 getter로 옮기기만 한 것 같아서 제가 한 방식이 브라이언의 의도가 맞는지 궁금합니다!
| public boolean isPrev() { | ||
| return prev; | ||
| } | ||
|
|
||
| public boolean isNext() { | ||
| return next; | ||
| } | ||
|
|
There was a problem hiding this comment.
has를 사용하면 handlebars에서 읽지 못하여 일단 is로 남겨두었습니다 ㅠㅠ
| } | ||
|
|
||
| public int notDeletedSize() { | ||
| return questionRepository.findAllByDeletedIsFalse().size(); |
|
@ehdrhelr 안녕하세요. 어떻게 좀 잘 돼가고 계신가요? 머지할까요? 아니면 좀 더 둘까요? |
|
@wheejuni 헉 죄송합니다... 미션5 수정하고 웹서버 미션하느라 정신이 없어서 신경 쓰지 못하고 있었습니다 ㅠㅠ |
|
@ehdrhelr ㅎㅎ 죄송하시지 않으셔도 돼요 ㅠㅠ 힘내세요!! |
1. js 메서드 분리 2. countOfAnswer 기본형으로 변경 3. 변수명, 클래스명 의미있게 변경 4. 파일 마지막줄 공백 추가 5. 디미터의 법칙 적용
JavaScript ES6 적용
1. 상황에 맞는 custom 예외를 정의하고 ControllerAdvice를 통해 예외처리 2. 댓글 수정페이지에서 질문 작성자와 시간 올바르게 표시
[시온] step 5. XHR과 AJAX, 제출합니다.
1. cri -> criteria 2. repository에 count 추가 3. service에서 작성자 일치여부 검증
…p6에 rebase하였고, step5 머지된 이후 step5에서 추가로 개선된 코드를 step6에 적용 1. commonApi() -> api() 2. 도메인.isWrittenBy(User) 에서 예외 throw 3. IdAndBaseTimeEntity -> BaseTimeEntity 4. HttpSessionUtils.isLoginUser(session)이 HttpSessionUtils.getUserFromSession(session)에서 내부적으로 시행되므로 두 메서드가 중복된 경우 isLoginUser를 제거 5. deleted와 password에 JsonIgnor추가하여 노출 금지 6. 비 로그인 상태에서 댓글 작성 및 삭제시 alert창 대신 예외 발생시켜 로그인화면으로 이동 7. NotFoundException 세분화하여 (User, Question, Answer) 간단한 코드 구현 8. hasPrev, hasNext를 사용하면 handlebars에서 읽지 못하여 isPrev, isNext 변 9. 예외를 주로 도메인, utils에서 throw
…p6을 rebase하였고, 이후 step5에서 추가로 개선된 코드 local/temp에 저장한 후 step6에서 local/temp를 merge 하였음 1. commonApi() -> api() 2. 도메인.isWrittenBy(User) 에서 예외 throw 3. IdAndBaseTimeEntity -> BaseTimeEntity 4. HttpSessionUtils.isLoginUser(session)이 HttpSessionUtils.getUserFromSession(session)에서 내부적으로 시행되므로 두 메서드가 중복된 경우 isLoginUser를 제거 5. deleted와 password에 JsonIgnor추가하여 노출 금지 6. 비 로그인 상태에서 댓글 작성 및 삭제시 alert창 대신 예외 발생시켜 로그인화면으로 이동 7. NotFoundException 세분화하여 (User, Question, Answer) 간단한 코드 구현 8. hasPrev, hasNext를 사용하면 handlebars에서 읽지 못하여 isPrev, isNext 변 9. 예외를 주로 도메인, utils에서 throw
|
피드백이 많이 늦었습니다.. step5를 rebase하는 과정에서 커밋메세지도 많이 꼬였군요 ㅠㅠ |
안녕하십니까, 시온입니다.
미션5가 머지되기 전에 PR을 open해서 미션5 커밋이 남아있습니다.
미션6은 89c8659 [test : Paging TDD를 위한 테스트 코드 작성] 부터 시작합니다!
특이사항은 아래와 같습니다.
감사합니다.