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

[Spring 지하철 노선도 - 1,2단계] 레넌(조형래) 미션 제출합니다. #212

Merged
merged 35 commits into from
May 10, 2022

Conversation

brorae
Copy link

@brorae brorae commented May 4, 2022

안녕하세요, 럿고!
이번 미션 리뷰 잘 부탁드립니다!

질문사항

  1. 테스트와 관련해서 패키지 별, 클래스 별로 테스트 코드를 돌렸을 때는 정상 작동하는데, 전체 테스트를 수행했을 때는 에러가 발생합니다.
    SubwayApplicationTests와 함께 테스트를 돌리면 에러가 발생하는 것 같습니다. SubwayApplicationTests의 annotation을 JdbcTest로 바꾸면 정상작동 하는 것을 확인하고 고민해보았는데, 이유를 찾지 못했습니다. 왜 에러가 나는걸까요?

에러가 나는 상황

스크린샷 2022-05-04 오후 5 12 22

패키지 별로 돌렸을 때 정상 작동

스크린샷 2022-05-04 오후 5 12 47

-> application.yml 파일 수정하여 해결하였습니다.

  1. 서비스 관련 질문
    서비스는 도메인 로직을 수행하는 역할이라고 생각합니다. 하지만 현재 상황에서는 도메인 로직이 검증을 수행하는 정도밖에 존재하지 않아서, 서비스를 만들지 않아도 된다고 생각했는데, 지금과 같은 상황에서도 서비스 레이어를 만드는 것이 좋을까요?

Copy link

@ksy90101 ksy90101 left a comment

Choose a reason for hiding this comment

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

레넌 안녕하세요 :) 리뷰어 럿고입니다.
지하철 미션 하시느라 고생 많으셨습니다 💯
간단한 피드백을 좀 더 남겼으니, 참고해주시면 감사하겠습니다~
질문은 언제든지 DM으로 연락주세요!

src/main/java/wooteco/subway/dao/LineDao.java Outdated Show resolved Hide resolved
src/main/java/wooteco/subway/dao/LineDao.java Outdated Show resolved Hide resolved
src/main/java/wooteco/subway/dao/LineDao.java Outdated Show resolved Hide resolved
src/main/java/wooteco/subway/dao/StationDao.java Outdated Show resolved Hide resolved
src/main/java/wooteco/subway/domain/Line.java Outdated Show resolved Hide resolved
src/main/java/wooteco/subway/ui/LineController.java Outdated Show resolved Hide resolved
src/main/java/wooteco/subway/ui/LineController.java Outdated Show resolved Hide resolved
@ksy90101
Copy link

ksy90101 commented May 5, 2022

서비스 관련 질문
서비스는 도메인 로직을 수행하는 역할이라고 생각합니다. 하지만 현재 상황에서는 도메인 로직이 검증을 수행하는 정도밖에 존재하지 않아서, 서비스를 만들지 않아도 된다고 생각했는데, 지금과 같은 상황에서도 서비스 레이어를 만드는 것이 좋을까요?

서비스가 단순히 도메인 로직을 수행하는 것만 있을까요? validate도 잇지 않을까요?
또한 클라이언트와 데이터 계층의 중간에서 의존성을 줄이는 역할도 한다고 생각이 들어요.
아래 글을 한번 읽어보시면 좋을거 같기도 합니다 :)
https://martinfowler.com/eaaCatalog/serviceLayer.html
추가적으로 좀 더 이야기를 하고 싶다면 언제든지 연락주세요~

Copy link

@ksy90101 ksy90101 left a comment

Choose a reason for hiding this comment

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

안녕하세요 레넌 :)
리뷰 적용하시느라 고생 많으셨습니다 💯
간단한 피드백을 좀더 남겼으니 조금만 더 파이팅 해보세요!
언제든지 질문은 DM으로 연락주세요~

src/main/java/wooteco/subway/service/LineService.java Outdated Show resolved Hide resolved
src/main/resources/application.yml Outdated Show resolved Hide resolved
src/test/java/wooteco/subway/dao/FakeLineDao.java Outdated Show resolved Hide resolved
src/test/java/wooteco/subway/dao/StationDaoTest.java Outdated Show resolved Hide resolved
src/test/java/wooteco/subway/dao/StationDaoTest.java Outdated Show resolved Hide resolved
src/main/java/wooteco/subway/service/StationService.java Outdated Show resolved Hide resolved
Copy link

@ksy90101 ksy90101 left a comment

Choose a reason for hiding this comment

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

레넌 안녕하세요 :)
피드백 적용하시느라 고생 많으셨습니다 💯
전체적으로 깔끔해진거 같아서, 이만 1,2단계는 머지할게요~
3단계때 다시 뵙겠습니다!

@ksy90101 ksy90101 merged commit d26ecdc into woowacourse:brorae May 10, 2022
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.

3 participants