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

[쿠킴] 스프링 카페 3단계 - DB에 저장하기 #109

Merged
merged 19 commits into from Mar 19, 2022

Conversation

ku-kim
Copy link
Member

@ku-kim ku-kim commented Mar 16, 2022

안녕하세요~ 리뷰어님! 쿠킴이라고 합니다.
Step3 요구사항 구현하여 리뷰 요청드립니다.
미리 리뷰 감사드립니다.😆


Heroku 배포 사이트

환경 : Java11 + Spring boot + Gradle + H2 DB embedded+ Heroku

View

main
users

테스트

test

- 박싱타입에서 기본 타입으로 변경
- 클라이언트 GET "/articles/{articleId}" 요청을 통해 입력받는 articleId도 int로 변경
…cle 으로 변경

- 회원가입 폼을 리턴하는 API였지만 이전 createArticle 메서드는 url과 매칭되지 않기 때문에 이름 변경
- 같은 아이디인지 체크하는 메서드 명의 가독성을 위해 변경
- 기존 박싱타입 데이터 -> int로 변경했기 때문에 Objects.equals() -> == 으로 값비교 변경
- 같은 아이디인지 체크하는 메서드 명의 가독성을 위해 변경
…)로 변경

- 기존 선형 탐색을 배열의 순서로 바로 접근하도록 변경
- any() 대신 자세한 값으로 변경 (line 33)
- 중복된 불필요한 테스트 제거 (line37)
- 개행 추가로 가독성 향상 (line46 ~ 49)
…ArticleWriteRequest.toDomain() 를 통해 Article 생성하도록 변경)

- 도메인이 DTO 참조하고 있는 부분 제거
- 연관되어 있는 구현 코드와 테스트 코드 모두 변경
- view 관련 팡리 파일 끝 개행 추가
- build.gradle에 H2, JDBC 의존성 추가
- application.properties에 H2 환경설정과 디버깅 모드 추가
  - 콘솔 사용, embedded 모드 설정, 사용자 id, pw 설정
  - logging.level.web 추가
- application.properties의 spring.sql.init.mode=always를 통해 resources에 있는 schema.sql, data.sql 초기 실행되도록 옵션 설정
- schema.sql에서 User, Article 테이블 생성 (DDL)
- data.sql에서 User, Article 더미 데이터 추가 (DML)
…조 Map -> List로 변경

- User 객체 생성 시 생성자 매개변수 UserJoinRequest 받는 것을 제거
- MemoryUserRepository 불필요한 Long id 제거, 요구사항에 맞는 Map -> List로 변경
- Long id 삭제로 view에 id 번호 삭제
- JdbcTemplate + h2 database 사용하여 리포지토리 구현
- embedded db로 통합 테스트
… 구현과 테스트

- JdbcTemplate + h2 database 사용하여 리포지토리 구현
- embedded db로 테스트 (JdbcTest)
- system.properties : heroku 배포를 위해 자바 11 명시
- build.gradle : heroku에서 실행할 하나의 jar만 생성되도록 plain.jar 파일 빌드 자동생성 옵션 끄기
- GET "/" 요청 시 동적 리소스 /index 위치 -> index로 변경
- mustache 경로 수정 "/component" -> "component"
- 테스트 코드의 클래스명 오타 수정
@ku-kim ku-kim added the review-BE Improvements or additions to documentation label Mar 16, 2022
@wheejuni wheejuni self-requested a review March 19, 2022 06:58
@wheejuni wheejuni self-assigned this Mar 19, 2022
Copy link

@wheejuni wheejuni left a comment

Choose a reason for hiding this comment

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

전반적으로 큰 문제 없습니다 💯
간단히 생각해볼만한 거리들을 남겨드렸는데 한번씩만 확인 쓱 해보세요~

@@ -5,20 +5,20 @@

public class Article {

private Integer id;
private int id;

Choose a reason for hiding this comment

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

PK에 해당하는 필드의 자료형을 int로 하는 것에 대한 위험성을 충분히 인지하고 계시나요?
(int 는 몇까지 표현할 수 있나요?)

Copy link
Member Author

Choose a reason for hiding this comment

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

네, 조언해주신 것 처럼 int는 4byte 크기로 음수를 사용하지 않을 약 21억까지 사용할 수 밖에 없네요.
큰 데이터 범위를 고려하겠습니다. 감사합니다.!

Comment on lines +26 to +28
public Article toDomain() {
return new Article(writer, title, contents);
}

Choose a reason for hiding this comment

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

표현 계층에 해당하는 클래스에서, 훨씬 하위에 있는 도메인 클래스에 강하게 결합하는 로직인데요, 괜찮을지 모르겠습니다.
오히려 Article 측에서 .fromWriteRequest(ArticleWriteRequest request) 와 같은 메소드 있었다면 어땠을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

네, 말씀해주신 것 처럼 표현 계층, 사용자 입력 포맷을 매핑한 클래스(ArticleWriteRequest)입니다.

  1. 저장 상황
    현재 사용자의 입력에 따라 Article를 저장한다면
    컨트롤러에서 사용자 입력을 받고 서비스 계층으로 ArticleWriteRequest 자체를 넘기고 서비스 계층에서 Article 도매인 객체를 생성해 저장하고 있습니다.
    Q) Article 도메인 객체 생성할 때 ArticleWriteRequest.todomain()으로 생성하고 있는데 이것이 강하게 결합된다는 말씀으로 이해했습니다. 그렇다면 어떻게 도메인 객체를 생성하는 것이 좋을까요? 처음엔 new Article(ArticleWriteRequest)로 구현했으나 도메인 객체가 ArticleWriteRequest를 강하게 결합하고 있는 문제가 있어 ArticleWriteRequest에서 도메인 객체를 생성하는 toDomain() 메서드를 만들었습니다. new Article(ArtcleWriteRequest.getUserId, ....) 의 게터로 넣는 것이 좋을까요?

  2. 조회 상황
    만약 Article 도메인 객체를 조회하는 사용자 요청이 오면 ArticleWriteRequest를 뷰에 뿌려주는 것이 아닌 도메인 객체 Article 자체를 넘기고 있습니다. 말씀처럼 도메인을 표현 계층에 바로 넘기는 것 보다 표현 계층의 매핑한 클래스(ArticleWriteRequest)를 뷰에 뿌려줘야겠다고 생각이 들었습니다. 감사합니다.!

@wheejuni wheejuni merged commit 11c263b into codesquad-members-2022:ku-kim Mar 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-BE Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants