-
Notifications
You must be signed in to change notification settings - Fork 178
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
[1, 2단계 - Spring 적용하기] 다니(이다은) 미션 제출합니다. #234
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반갑습니다 다니ㅎㅎㅎ
매번 새로운 환경에 적응하시느라 힘드실텐데 잘 적응하시네요 👍
질문을 포함해서 몇가지 코멘트 남겼으니 궁금한점이 있으시면 언제든 DM주세요~!
} | ||
|
||
@GetMapping("/start") | ||
public String start() throws SQLException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
왜 SQLException을 catch하려고 하셨을까요 🤔
여기부터 하위 메서드의 throws SQLException
을 차근차근 지워나가보세요~
그리고 정상적으로 작동한다면 왜 그럴까도 고민해보시길 바랍니다ㅎㅎ
} | ||
|
||
@GetMapping("/chess") | ||
public String chess(final Model model) throws SQLException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 메서드에서 service 호출을 무려 10번을 하고있습니다!
우선 컨트롤러는 단순히 클라이언트와 연결하는 역할입니다.
로직이나 절차는 최소화하여 서비스나 도메인으로 옮겨볼 수도 있고, 나눠볼 수도 있겠네요 ^_^
build.gradle
Outdated
@@ -22,6 +22,8 @@ dependencies { | |||
implementation 'net.rakugakibox.spring.boot:logback-access-spring-boot-starter:2.7.1' | |||
implementation 'pl.allegro.tech.boot:handlebars-spring-boot-starter:0.3.1' | |||
|
|||
implementation 'com.google.code.gson:gson:2.8.5' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gson보다 spring을 적극 이용하셨으면 좋겠네요ㅎㅎ
필요하다면 DTO를 만들거나 api를 추가로 만들어서 가져오는건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
930623f
기존에 사용하던 Gson을 없애고 Jackson으로 Object를 json 형식으로 변환했습니다!
|
||
@PostMapping(value = "/turn", produces = "application/json") | ||
@ResponseBody | ||
public void turn(@RequestBody TurnChangeRequestDto turnChangeRequestDto) throws SQLException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떤 결과를 내려주면 클라이언트에서 좀 더 처리하기 용이할것 같네요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1f3c36b
이렇게 HTTP 상태코드를 반환하도록 변경했는데 괜찮은 방법인가요??
} | ||
|
||
@PostMapping(value = "/move", produces = "application/json") | ||
@ResponseBody |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RestController와 @controller의 차이를 학습해보고 필요하다면 분리해보시는 것도 좋을 것 같네요~
new ArrayDeque<>(Arrays.asList("move", moveRequestDto.getSource(), moveRequestDto.getTarget())); | ||
try { | ||
executeRound(commands); | ||
} catch (RuntimeException runtimeException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exception을 정리한다면, @ControllerAdvice를 통해 보다 쉽게 에러처리를 하실 수 있을거에요
} | ||
|
||
@Override | ||
public Map<String, String> filteredChessBoard(final Map<Position, Piece> chessBoard) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정리가 필요하겠네요 🤔
@@ -0,0 +1,23 @@ | |||
package chess.dto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
request, response인 dto와 그렇지 않은 dto의 기준은 무엇인가요~?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- dto/RequestDto: 인자로 DTO가 들어가는 경우
- dto/ResponseDto: 반환값으로 DTO가 나오는 경우
- dto: 인자로 DTO가 들어가고, 반환값으로 DTO가 나오는 경우
이 기준으로 분리해서 작성했습니다! 혹시 네이밍에 문제가 있을까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
없습니다! 👍
Map<String, String> chessBoardFromDB() throws SQLException; | ||
|
||
Map<Position, Piece> chessBoard(final Map<String, String> chessBoardFromDB) throws SQLException; | ||
|
||
Map<Position, Piece> chessBoard(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
객체로 포장이 가능하다면 해보셔도 좋을 것 같습니다
return "chess"; | ||
} | ||
|
||
@PostMapping(value = "/move", produces = "application/json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PostMapping(value = "/move", produces = "application/json") | |
@PostMapping(value = "/move", produces = MediaType.APPLICATION_JSON_VALUE) |
요런 친구도 있으니 참고해주세요 ^_^
안녕하세요 김고래! 다니입니다 🙌 남겨주신 피드백 바탕으로 리팩토링 진행했습니다! 이번에 리팩토링하면서 새로 알게되는 지식이 많았어요! 일부 코멘트들에는 제 의견을 달았는데요. 확인해주시면 감사하겠습니다! 이번에도 코드 리뷰 잘 부탁드립니다! |
📚 다니의 학습 로그[Spring] @controller vs @RestController - 3내용
링크
[Spring] WebMvcConfigurer: addViewControllers() - 3내용
링크[Spring] @ControllerAdvice (+ @ExceptionHandler) - 4내용
링크
[Java] Jackson: Json <-> Object 변환 - 2내용
링크
[Web] HTTP 상태코드 - 2내용
링크 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1,2단계 진행하시느라 고생하셨습니다 👍
코멘트 참고하여 다음 단계를 진행해주세요~!
ChessBoardDto chessBoard = chessService.chessBoardFromDB(); | ||
String jsonFormatChessBoard = chessService.jsonFormatChessBoard(chessBoard); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기는 제가 혼란을 드린것 같네요ㅠㅠ
gson을 제거하면서 api로 객체를 리턴하는게 어떨까해서 코멘트를 남겼었습니다...ㅎㅎ;
일반적으로 controller는 페이지 이동, restController는 api를 사용해 데이터를 넘겨주는 역할을 가집니다
import java.util.List; | ||
|
||
@Repository | ||
public class PieceDao extends DBConnection { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DBConnection은 없어도 될것같네요 :)
|
||
@Repository | ||
public class PieceDao extends DBConnection { | ||
@Autowired |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Autowired 도요 ^_^
try { | ||
jdbcTemplate.update(query, pieceName, piecePosition); | ||
} catch (Exception e) { | ||
e.printStackTrace(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try catch 도요!
|
||
public List<ChessResponseDto> showAllPieces() { | ||
List<ChessResponseDto> pieces = new ArrayList<>(); | ||
String query = "SELECT * FROM piece"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재는 잘 작동하겠지만, select건 update, delete건 where절은 항상 붙여야 문제가 덜 발생한다는 점 참고해주세요ㅎㅎ
@Service | ||
public class ChessService { | ||
private final ChessRepository chessRepository; | ||
private Round round; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 친구는 다음 단계를 진행하시면서 자연스럽게 없어질것 같은데 thead safe한 방향을 고민해봐주세요^_^
안녕하세요, 김고래! 다니입니다 🙌
레벨 1 첫 미션 때 뵙고 오랜만이에요 !
페어인 @taewankimmmm과 스프링 학습 테스트를 먼저 진행하고,
체스를 Spark에서 Spring으로 열심히 옮겨봤어요!
이번 미션을 하면서 궁금한 점이 하나 생겼는데 질문 드려도 괜찮을까요?!
Q.
기존 DB 로직을 사용할 때는 try-catch의 catch에서 SQLException을 파라미터로 넘겨줬을 때 문제가 없었어요.
근데, JDBC로 변경하면서 SQLException을 넘기면 에러가 뜨고 Exception을 넘겨줘야 문제가 없더라구요!
SQLException을 넘겼을 때 뜨는 에러 메시지를 읽어보면 SQLException은 아예 못 던진다고 나오는데요.
왜 JDBC에서는 SQLException을 사용하지 못하는 건가요?? 이유가 궁금합니다...!!
이번에도 코드 리뷰 잘 부탁드립니다 !
부족한 점 마구마구 지적해주세요~!
귀한 시간 내주셔서 감사합니다 🙇♀️❣️