Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hiro & bongf] Mission6 페이지나누기 #134

Open
wants to merge 8 commits into
base: bong6981
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions src/main/java/com/codessquad/qna/web/QuestionPage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package com.codessquad.qna.web;

import com.codessquad.qna.web.domain.Question;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;

import java.util.ArrayList;
import java.util.List;

public class QuestionPage {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체로 포장하려는건 좋은 시도였어요~

private final int COUNT_NUMBERS_TO_SHOW = 5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private final int COUNT_NUMBERS_TO_SHOW = 5;
private final static int COUNT_NUMBERS_TO_SHOW = 5;

private final Pageable pageable;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

얘는 final이고 나머지는 final이 아닌 이유가 있을까요?

private int previous;
private int next;
private Page<Question> questions;
private List<Integer> pageNumbers = new ArrayList<>(COUNT_NUMBERS_TO_SHOW);
Comment on lines +13 to +16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 상태로 관리되는 값과 아닌 값을 구분하는 시야를 기르셨으면 좋겠네요.
단순히 데이터를 저장하는 목적으로 필드를 사용하는 것은 추천드리지 않습니다.


public QuestionPage(Pageable pageable, Page<Question> questionsByPage) {
this.pageable = pageable;
this.questions = questionsByPage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

가급적이면 일치시켜주세요~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 일치시키도록 하겠습니다!

this.previous = questionsByPage.previousOrFirstPageable().getPageNumber();
if (questionsByPage.hasNext()) {
this.next = questionsByPage.nextPageable().getPageNumber();
} else {
this.next = questionsByPage.getNumber();

}
this.pageNumbers = createPageNumbers();
Comment on lines +20 to +28
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 유도된 친구들은 그냥 메서드 호출하면 그 때 반환해주는건 어때요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오! 그런 방식은 생각해보지 않았네요. 그렇게 되면 멤버변수에 대한 고민도 해결되는 부분이 있는 것 같습니다! 감사합니다. 위에 final 키워드와 함께 수정해보겠습니다!

}

private List<Integer> createPageNumbers() {
int currentPage = questions.getNumber();
int totalPages = questions.getTotalPages();
int startNumber = (currentPage / COUNT_NUMBERS_TO_SHOW) * COUNT_NUMBERS_TO_SHOW;
int endNumber = startNumber + COUNT_NUMBERS_TO_SHOW - 1;

for (int i = startNumber; i <= endNumber; i++) {
pageNumbers.add(i);
if (i == totalPages - 1) {
break;
}
}
return pageNumbers;
}

public Pageable getPageable() {
return pageable;
}

public int getPrevious() {
return previous;
}

public int getNext() {
return next;
}

public Page<Question> getQuestions() {
return questions;
}

public List<Integer> getPageNumbers() {
return pageNumbers;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public ApiAnswerController(AnswerService answerService) {

@PostMapping
public Answer createAnswer(@PathVariable long questionId, String contents, HttpSession session) {
User user = HttpSessionUtils.getSessionedUser(session).orElseThrow(NotLoginException::new);
User user = HttpSessionUtils.getSessionedUser(session).orElseThrow(NotLoginException::new);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
User user = HttpSessionUtils.getSessionedUser(session).orElseThrow(NotLoginException::new);
User user = HttpSessionUtils.getSessionedUser(session).orElseThrow(NotLoginException::new);

메서드 오퍼레이터의 사용은 좋네요!

return answerService.postAnswer(user, questionId, contents);
}

Expand All @@ -35,17 +35,17 @@ public Result deleteAnswer(@PathVariable("questionId") long questionId, @PathVar
}

@ExceptionHandler(IllegalEntityIdException.class)
public String handleIllegalEntityIdException(IllegalEntityIdException e) {
return e.getMessage();
public String handleIllegalEntityIdException() {
return "유효하지 않은 접근입니다";
}

@ExceptionHandler(NotLoginException.class)
public String handleNotLoginException(NotLoginException e) {
return e.getMessage();
public String handleNotLoginException() {
return "로그인이 필요합니다";
}

@ExceptionHandler(IllegalAccessException.class)
public String handleIllegalAccessException(IllegalAccessException e) {
return e.getMessage();
public String handleIllegalAccessException() {
return "자신의 답변만 삭제할 수 있습니다";
}
Comment on lines 37 to 50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이전 리뷰어분이 이런 형식의 예외 핸들링을 바라신 것은 아닌 것 같아요.
저는 솔직히 아직 이정도 수준의 예외 처리를 바라진 않는데, 에러를 반환하는 객체를 정의해서 일정한 형식으로 주는것을 고민해보세요!

}
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package com.codessquad.qna.web.controller;

import com.codessquad.qna.web.HttpSessionUtils;
import com.codessquad.qna.web.QuestionPage;
import com.codessquad.qna.web.domain.Question;
import com.codessquad.qna.web.domain.User;
import com.codessquad.qna.web.exception.NotLoginException;
import com.codessquad.qna.web.service.QuestionService;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.web.PageableDefault;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.*;
Expand Down Expand Up @@ -36,8 +40,9 @@ public String createQuestion(Question question, HttpSession session) {
}

@GetMapping
public String getQuestions(Model model) {
model.addAttribute("questions", questionService.findQuestions());
public String getQuestions(@PageableDefault(size = 15, sort = "createdDateTime", direction = Sort.Direction.DESC) Pageable pageable, Model model) {
QuestionPage questionPages = questionService.getQuestionPage(pageable);
model.addAttribute("pages", questionPages);
return "/index";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index로 가는거 맞을까요~?

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
@MappedSuperclass
@EntityListeners(AuditingEntityListener.class)
public class AbstractEntity {
private static final DateTimeFormatter QUESTION_DATETIME_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm");
private static final DateTimeFormatter DATETIME_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저라면 QNA_DATETIME_FORMAT 정도로 해줬을 것 같네요~


@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
Expand Down Expand Up @@ -57,7 +57,7 @@ private String getFormattedDate(LocalDateTime dateTime) {
if (dateTime == null) {
return "";
}
return dateTime.format(QUESTION_DATETIME_FORMAT);
return dateTime.format(DATETIME_FORMAT);
}
}

Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package com.codessquad.qna.web.repository;

import com.codessquad.qna.web.domain.Question;
import org.springframework.data.repository.CrudRepository;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;

public interface QuestionRepository extends CrudRepository<Question, Long> {
public interface QuestionRepository extends JpaRepository<Question, Long> {
Page<Question> findAll(Pageable pageable);
}
18 changes: 13 additions & 5 deletions src/main/java/com/codessquad/qna/web/service/QuestionService.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.codessquad.qna.web.service;

import com.codessquad.qna.web.HttpSessionUtils;
import com.codessquad.qna.web.QuestionPage;
import com.codessquad.qna.web.domain.Question;
import com.codessquad.qna.web.domain.User;
import com.codessquad.qna.web.exception.IllegalAccessException;
import com.codessquad.qna.web.exception.IllegalEntityIdException;
import com.codessquad.qna.web.repository.QuestionRepository;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;

@Service
Expand All @@ -21,10 +23,6 @@ public void postQuestion(Question question, User user) {
questionRepository.save(question);
}

public Iterable<Question> findQuestions() {
return questionRepository.findAll();
}

public Question findQuestion(long id) {
return questionRepository
.findById(id)
Expand Down Expand Up @@ -53,4 +51,14 @@ private void checkSameWriter(Question question, User user) {
throw new IllegalAccessException();
}
}

public Page<Question> getQuestionsByPage(Pageable pageable) {
return questionRepository.findAll(pageable);
}

public QuestionPage getQuestionPage(Pageable pageable) {
return new QuestionPage(pageable, getQuestionsByPage(pageable));
}


}
10 changes: 10 additions & 0 deletions src/main/java/temp.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
public class temp {
public static void main(String[] args) {
for(int i=1; i<=200; i++) {
System.out.println(
"INSERT INTO QUESTION (ID, WRITER_ID, TITLE, CONTENTS, CREATED_DATE_TIME, COUNT_OF_ANSWERS) VALUES ('"+ i + "', '1', '6', 'dui', '2021-03-20 16:03:03.106461', '1')"
);
}

}
Comment on lines +3 to +9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 이건 아닌데요~
스프링에서는 스프링 컨테이너 시작시점에 동작을 수행하는 기능도 존재합니다. 이를 이용해보세요.

}
Loading