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

feat: implement stock domain #5

Merged
merged 8 commits into from
Jan 21, 2024
Merged

feat: implement stock domain #5

merged 8 commits into from
Jan 21, 2024

Conversation

songyi00
Copy link
Member

@songyi00 songyi00 commented Jan 20, 2024

Issue Number

close: #3

작업 내역

구현 내용 및 작업 했던 내역

  • stock domain 구현
  • stock repository 구현
  • application 설정 파일 분리

Copy link
Member

@chominho96 chominho96 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 같이 맞춰보면 좋을만한 부분 리뷰 남겨놓았어요!


private double price;

private Double volume;
Copy link
Member

Choose a reason for hiding this comment

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

거래량은 정수로 표현될 수 있다고 생각하는데 double형을 쓰신 이유가 있을까요???

Copy link
Member Author

Choose a reason for hiding this comment

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

@chominho96 혹시 몰라서 일단 실수형으로 넣어놓았어요! API 스펙 확인했는데 정수로 바꿔도 될 것 같습니다 :)

Copy link
Member

Choose a reason for hiding this comment

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

거래량은 화면에서 표현도 정수형으로 될거 같아서 저는 정수형으로 저장되어도 괜찮다고 생각해요!!


private String industry;

private double price;
Copy link
Member

Choose a reason for hiding this comment

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

null 체크가 되려면 Double 형이 맞을거 같은데 어떻게 생각하시나요??

Copy link
Member Author

Choose a reason for hiding this comment

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

@chominho96 오 이건 오타(?)입니다. 수정해놓을게요!!

RealEstate,
Energy,
Utilities,
ETC
Copy link
Member

Choose a reason for hiding this comment

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

약간 관례?적으로 enum에서 상수는 모두 대문자로 표기한다고 알고 있어서 이 부분 같이 맞춰보면 좋을거 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

-- 논의 및 수정 완료 --

@chominho96 chominho96 self-requested a review January 20, 2024 11:13
Copy link
Member

@chominho96 chominho96 left a comment

Choose a reason for hiding this comment

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

Stock 도메인 개발 수고하셨습니다~!

@songyi00 songyi00 merged commit d6c6f86 into develop Jan 21, 2024
1 check passed
@songyi00 songyi00 added the enhancement New feature or request label Jan 23, 2024
@chominho96 chominho96 deleted the feat/#3 branch February 19, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants