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

[1, 2단계 - Spring 적용하기] 인비(김태희) 미션 제출합니다. #218

Merged
merged 54 commits into from Apr 19, 2021

Conversation

taehee-kim-dev
Copy link

@taehee-kim-dev taehee-kim-dev commented Apr 13, 2021

안녕하세요, 김고래!! 🐳

처음 뵙네요!! 반갑습니다. 😄

이번엔 Spring을 적용하는 미션이었습니다.

순수 자바로 개발했던 애플리케이션을 차근차근 Spring으로 전환해야 하는 게 쉽지 않았어요.

하지만 스프링 코드가 왜 그렇게 생겼고, 어떻게 작동하는지에 대해 몸소 느낄 수 있어서 정말 좋았어요. 👍 👍

실행 안내 사항

  • 사전에 MySQL에서 jwp-chess/chess_game.sql 파일의 명령문들을 맨 위에서부터 맨 아래까지 차례대로 실행해야 합니다. 이 과정을 먼저 마쳐야, 애플리케이션을 실행하거나 테스트할 수 있습니다.

  • 애플리케이션 실행 방식을 웹, 또는 콘솔로 전환하려면, application.properties 파일에서 해당 profileactive 시켜주시면 됩니다. (파일을 보면 이해되실 거예요!)


브라운이 공지한 대로, 레벨2부터 학습 로그는 제 블로그에 포스팅하고 해당 링크를 공유할게요.

이번 미션을 하면서 정리한 학습 로그는 아래와 같습니다!
우아한테크코스 3기 백엔드 Lv2 [체스 - 1, 2단계] 1차 PR 학습로그

소중한 시간 내어 리뷰해 주셔서 감사합니다. 🙇

ps. 혹시 애플리케이션 실행이나 테스트에서 문제점이 있으면 언제든 DM 부탁드려요!!

Copy link

@ep1stas1s ep1stas1s left a comment

Choose a reason for hiding this comment

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

안내사항까지... 감사합니다 ^_^
UI도 깔끔하게 잘 적용해주셨네요 👍
Spring 위주로 코멘트 남겼으니 참고해보시고 궁금한점은 DM주세요~!

import org.springframework.stereotype.Repository;

@Repository
public class ChessGameDAO implements ChessGameRepository {

Choose a reason for hiding this comment

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

Suggested change
public class ChessGameDAO implements ChessGameRepository {
public class ChessGameDao implements ChessGameRepository {

약어가 연속해서 나오면 가독성을 해칠 수 있기 때문에, 동일하게 카멜케이스를 적용해주세요 ^_^

this.jdbcTemplate = jdbcTemplate;
}

private final RowMapper<ChessGameEntity> ChessGameEntityRowMapper = (resultSet, rowNum) ->

Choose a reason for hiding this comment

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

Suggested change
private final RowMapper<ChessGameEntity> ChessGameEntityRowMapper = (resultSet, rowNum) ->
private final RowMapper<ChessGameEntity> chessGameEntityRowMapper = (resultSet, rowNum) ->

@@ -0,0 +1,6 @@
spring.profiles.active=web

Choose a reason for hiding this comment

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

profiles 적용 👍

  1. 웹과 콘솔을 나눠서 적용하셨으니, 테스트도 별개로 적용해보시는건 어떨까요??
    현재 구조에선 테스트를 진행하면 실제 사용중인 db에 데이터를 적재했다가 삭제해요
    테스트도 좋지만 실제 운영에 영향을 주면 안되겠죠?? 그렇기 때문에 다양한 방법으로 실제 DB를 대체하곤 해요
    그 방법 중 하나인 H2 db를 적용해서 테스트를 진행해보셔도 좋을 것 같습니다 :)

  2. 좀 더 확실한 구조를 위해 콘솔환경은 spring context를 안쓰시는건 어떨까요? 요건 꼭 수정하시라기 보단, 이런 마인드로 임하시면 좀 더 명확하게 구조가 잡힐 수 있을 것 같아 남김니다ㅎㅎ

import org.springframework.stereotype.Component;

@Component
public class PiecesCache {

Choose a reason for hiding this comment

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

PiecesCahce는 스프링에서만 사용 가능할 것 같은데 도메인에 위치하는 것이 맞을까요 🤔

인비의 console -> spark 과정은 보지 못했지만, 도메인은 많이 바뀌지 않았을거에요 (혹은 그렇게 피드백을 받았을 것 같아요ㅎㅎ)
spark -> spring과정도 아예 같진 않더라도 최대한 비슷한 방식으로 진행해보시면 좋을 것 같아요~

);

@Override
public ChessGameEntity save(ChessGameEntity chessRoomEntity) {

Choose a reason for hiding this comment

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

어떤 DAO method는 XxxEntity를 반환하고, 어떤 메서드는 도메인을 반환하네요
혹시 나누신 기준이 있으실까요~?


@Profile("console")
@Service
public class ChessConsoleService {

Choose a reason for hiding this comment

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

계속 같은 내용인데요ㅎㅎㅎㅎ;
ConsoleService와 WebService가 나뉠 필요가 있을까요 🤔

}

@GetMapping(ROOT + DELETE)
public String deleteRequest(@RequestParam("id") Long gameId) throws SQLException {

Choose a reason for hiding this comment

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

SQLException이 컨트롤러까지 올라오는게 맞을까 한 번 고민해주세요!
(그리고 정말 throws SQLException를 작성 추가해야하는 것이 맞나까지요 😄 )

this.destination = destination;
}

public void setGameId(Long gameId) {

Choose a reason for hiding this comment

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

set을 사용하지 않고 구현해보는건 어떨까요~?

@@ -0,0 +1,73 @@
package chess.controller.dto.response;

public class ResponseDTO {

Choose a reason for hiding this comment

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

보다 명확한 클래스명을 지어주세요 ^_^

return CHESS_BOARD_VIEW;
}

private void putBoardRanksToModel(Model model, BoardResponseDTO boardResponseDTO) {

Choose a reason for hiding this comment

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

BoardResponseDTO에서 메서드에 해당하는 map을 반환해주어도 될것 같네요 :)

@taehee-kim-dev
Copy link
Author

taehee-kim-dev commented Apr 17, 2021

안녕하세요, 김고래!! 😄

김고래의 피드백을 적용하면서, 정말 많이 배우고 깨달았어요.

소중한 피드백 감사합니다. 🙇

김고래가 해주신 모든 피드백을 반영했습니다.

프로덕션 코드 실행 시 안내사항은 아래와 같아요!

Spring Web Application 실행 안내 사항

  • 사전에 MySQL에서 jwp-chess/chess_game.sql 파일의 명령문들을 맨 위에서부터 맨 아래까지 차례대로 실행해야 합니다. 이 과정을 먼저 마쳐야, WebApplication.java 를 실행할 수 있습니다.

테스트는 H2 DB를 적용해서 별도로 할 게 없어요!! 😆
콘솔 게임은 Spring을 아예 사용하지 않아서, profile 세팅을 하지 않아도 됩니다.
대신, 메인 애플리케이션 파일이 WebApplication.javaConsoleApplication.java 로 나뉘어 졌어요.

콘솔 게임은 말씀하신 대로 Controller만 있고, Service는 없어졌습니다!
Controller에서 바로 Domain에 접근하도록 했어요.

이전에는 DB 테이블 5개가 외래키를 통해 연관관계를 맺었었는데요.
제이슨도 이전 방식을 보고 "한 게임이 생성되면, 32개(전체 기물들의 개수)의 row가 생성되는 거네요?" 라는 의문을 제기했었어요.
그래서 이번에 테이블을 1개로 간소화 했습니다.
제가 너무 어렵게 돌아가려고 한 것 같아요 😅

이렇게 하니 구조나 코드가 훨씬 깔끔해졌습니다.

질문

  1. 스프린 빈으로 등록해야 하는 객체들은 어떤 객체들인가요??

이전에 제가 도메인 객체들까지 모두 스프링 빈으로 등록했다가, 김고래가 도메인은 테스트 시 매 번 스프링을 띄워야 하니, 순수한 객체로 두는게 좋다는 피드백을 하셨어요.

도메인 객체들을 모두 스프링 빈으로 등록하면, 생성자 의존성 주입을 통해 느슨한 결합도를 유지할 수 있을 것 같은데, 테스트 시 @SpringBootTest 를 붙여 스프링 자체를 매 번 띄워야 하는 단점을 말씀하셨어요!! (도메인 객체들도 모두 스프링 빈으로 등록해 두어도, 테스트 할 때는 직접 생성자 주입을 하면 @SpringBootTest 를 사용하지 않아도 되지만요.)

어떤 객체를 스프링 빈으로 등록해야 하는지 명확한 기준이 헷갈립니다.

스프링에서만 필요한 객체들을 빈으로 등록해야 하는건가요?? 기준이 궁금해요!!

아니면, ChessGame 객체 생성자의 매개변수로 Board 객체를 주입받는게 낮은 결합도를 위해 더 나은 형태일까요? Bean 등록을 통한 자동 주입 말고, 수동 주입이요!! 지금은 ChessGame 생성자 내부에서 new 를 통해 필드에 할당하는 방식이라 결합도가 높은 것 같아서요. 김고래의 생각은 어떠신가요?


피드백을 반영하고 정리한 학습로그는 아래에 있습니다.
우아한테크코스 3기 백엔드 Lv2 [체스 - 1, 2단계] 1차 코드리뷰 적용 학습로그


DM 질문 상세하게 잘 답변해주셔서 정말 감사합니다.

감사합니다!! 🙇

@ep1stas1s
Copy link

정말 많은 수정이 있었군요... 고생하셨습니다ㅎㅎㅎ
코드가 깔끔해진게 느껴지네요 💯 💯

스프린 빈으로 등록해야 하는 객체들은 어떤 객체들인가요??

일반적으로는 상태가 없는 객체를 싱글톤으로 사용하기 위해 빈으로 등록해요. 빈으로 등록한다는 의미는 개발자가 직접 생성과 제어를 관리하지 않고, 스프링이 그것을 맡아서 제어하죠. 그렇기 때문에 굳이 관리하지 않아도 되는 controller, service, repository가 대표적인 예가 될 수 있겠네요. 추가적으로 profile에 따라 달라진 설정 정보를 담고 있는 DataSource나 각종 property도 빈으로 등록하고 사용해요. 빈으로 등록했을 때 장/단점을 고려해보고 장점이 더 크다면 언제든 등록해도 좋다고 생각합니다^_^

@ep1stas1s ep1stas1s merged commit 6f9e695 into woowacourse:taehee-kim-dev Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants