-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
There was a problem hiding this 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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
거래량은 정수로 표현될 수 있다고 생각하는데 double형을 쓰신 이유가 있을까요???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chominho96 혹시 몰라서 일단 실수형으로 넣어놓았어요! API 스펙 확인했는데 정수로 바꿔도 될 것 같습니다 :)
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null 체크가 되려면 Double 형이 맞을거 같은데 어떻게 생각하시나요??
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
약간 관례?적으로 enum에서 상수는 모두 대문자로 표기한다고 알고 있어서 이 부분 같이 맞춰보면 좋을거 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-- 논의 및 수정 완료 --
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stock 도메인 개발 수고하셨습니다~!
Issue Number
close: #3
작업 내역