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

refactor(coin): coin 도메인 리팩토링 및 테스트 코드 작성 #99

Merged
merged 41 commits into from
May 3, 2024

Conversation

hw9402
Copy link
Contributor

@hw9402 hw9402 commented Apr 29, 2024

close #92

리팩토링도 리팩토링이지만 테스트 코드를 중점적으로 봐주시면 감사하겠습니다!
양이 많기도 했고 시간도 조금 부족했어서 하드 코딩한 부분들이 좀 있을 겁니다..

진행한 작업

  • 예외 처리를 클래스가 아닌 에러 코드를 주입하도록 변경했습니다
  • controller 패키지를 presentation 패키지로 이름을 변경했습니다
  • 서비스 레이어를 비즈니스 레이어와 구현 레이어로 분리했습니다
  • 조건에 따른 예외 처리를 Validator에서 하도록 변경했습니다
  • controller 및 service를 Command와 Query로 분리했습니다
  • 테스트 코드를 작성했습니다

이외에

코드 리뷰 진행 중 Read-only Transaction에서 save메서드를 사용한 기존 코드로 인해 아래와 같은 에러가 떴던 부분을 수정했습니다!

java.sql.SQLException: Connection is read-only. Queries leading to data modification are not allowed

마현우 added 29 commits April 24, 2024 19:43
@hw9402 hw9402 added the ✨ Feature There is Something to do label Apr 29, 2024
@hw9402 hw9402 added the 🔨 Refactor There is something to refactor label Apr 29, 2024
@hw9402 hw9402 self-assigned this Apr 29, 2024
@hw9402 hw9402 linked an issue Apr 29, 2024 that may be closed by this pull request
@hw9402 hw9402 changed the title coin 도메인 리팩토링 및 테스트 코드 작성 refactor(coin): coin 도메인 리팩토링 및 테스트 코드 작성 Apr 29, 2024
qlido
qlido previously approved these changes Apr 30, 2024
Copy link
Member

@qlido qlido left a comment

Choose a reason for hiding this comment

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

수고하셨습니다

@qlido
Copy link
Member

qlido commented Apr 30, 2024

TradeWithoutTradeStatusAndCoinAccountId 가 정확하지만 한번에 읽히지 않는다는 단점이 있는거 같아요.
주식의 호가처럼 갯수와 가격을 전달하는 역할을 하는 VO의 이름을 다시 정하는 것도 코드 가독성에 많은 도움이 될 것 같아요.

@NameIsUser06 NameIsUser06 self-requested a review April 30, 2024 10:01
@hw9402
Copy link
Contributor Author

hw9402 commented Apr 30, 2024

TradeWithoutTradeStatusAndCoinAccountId 가 정확하지만 한번에 읽히지 않는다는 단점이 있는거 같아요. 주식의 호가처럼 갯수와 가격을 전달하는 역할을 하는 VO의 이름을 다시 정하는 것도 코드 가독성에 많은 도움이 될 것 같아요.

다시 한 번 생각해보겠습니다!

Copy link
Member

@jacobhboy jacobhboy left a comment

Choose a reason for hiding this comment

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

굳입니다~ 정리 잘 하셨네요! 남은거 화이팅입니다!

Comment on lines +42 to +65
public List<Price> getPriceByPeriod(String period) {
switch (period) {
case "full" -> {
return priceReader.findAllByOrderByStartedTime();
}
case "halfMonth" -> {
LocalDateTime twoWeeksAgo = LocalDateTime.now().minusWeeks(2);
return priceReader.findAllAfterStartedTime(twoWeeksAgo);
}
case "week" -> {
LocalDateTime oneWeekAgo = LocalDateTime.now().minusWeeks(1);
return priceReader.findAllAfterStartedTime(oneWeekAgo);
}
case "day" -> {
LocalDateTime oneDayAgo = LocalDateTime.now().minusDays(1);
return priceReader.findAllAfterStartedTime(oneDayAgo);
}
case "halfDay" -> {
LocalDateTime halfDayAgo = LocalDateTime.now().minusHours(12);
return priceReader.findAllAfterStartedTime(halfDayAgo);
}
case "threeHours" -> {
LocalDateTime threeHoursAgo = LocalDateTime.now().minusHours(3);
return priceReader.findAllAfterStartedTime(threeHoursAgo);
Copy link
Member

Choose a reason for hiding this comment

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

항상 이런 코드를 볼 때 마다 느끼는 것인데, 더 좋은 방법이 없을까요?

@hw9402 hw9402 merged commit 6edce2f into master May 3, 2024
0 of 2 checks passed
@hw9402 hw9402 deleted the refactor/#92 branch May 3, 2024 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature There is Something to do 🔨 Refactor There is something to refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor(coin): Coin 도메인 리팩토링
4 participants