-
Notifications
You must be signed in to change notification settings - Fork 55
[정준영_BackEnd]1주차 과제 제출합니다. #62
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
base: main
Are you sure you want to change the base?
Conversation
asa9874
left a comment
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.
안녕하세요!
1주차 과제 고생하셨습니다!👍
몇가지 의견 코멘트로 남겨뒀습니다.😊
| } | ||
| return max; | ||
| } | ||
| } No newline at end of file |
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.
동일하게 EOF입니다
| public String getName() {return name;} | ||
|
|
||
| public int getPosition() {return position;} | ||
| } No newline at end of file |
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.
EOF 에 대해 알아보시면 좋을거같아요
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.
오 생각을 못해본 부분이었어요;; 덕분에 좋은 공부가 되네요. 다음부터는 신경 써볼게요!
|
|
||
| public void move() | ||
| { | ||
| if (Randoms.pickNumberInRange(0, 9) >= 4) |
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.
매직넘버의 개념을 처음 알았습니다;; 확실히 협업에서는 신경써야 하는 부분 같네요. 그러면 제 코드에서는 0과 9를 상수화 하는게 좋을까요 아니면 숫자 전부 다 하는 것이 좋을까요? 상수화 기준을 잡기가 모호하네요;;
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.
제 개인적인 생각이지만 매직넘버를 상수화 할때 숫자의 의도를 알수있는지 여부가 중요한거같아요
if(car.getCount() > 0){//차가 한대라도 있어야하는 조건이구나}처럼 0과같이 존재 여부, 인덱스 시작점 등 직관적인 숫자들은 매직넘버화할 필요가 없지만 다음과같은 경우에는 상수화가 필요할수있겠네요.
if(car.getCount() > 3){//차가 3대이상 있어야하는 조건? 3대가 뭘의미하지? }if(car.getCount() > MIN_RACING_CAR_COUNT){//아하 레이싱을 시작하기 위한 차 갯수구나}지금 코드에서 0~9중에 숫자 하나를 랜덤으로 뽑아서 4이상일때의 조건이라는것을 알수있지만 "왜 이 숫자인지" 0,9,4 라는 숫자가 해당 코드만 보고서는 그 의미를 알 수 없습니다.
if (Randoms.pickNumberInRange(0, 9) >= 4) //이게 어떤 코드인지는 알겠지만 왜 이런숫자가 들어왔는지 알수없음이처럼 비즈니스 로직에서 규칙,조건을 의미하는 숫자들은 상수화하는편이 좋은거같습니다.
| private int findMaxPosition() | ||
| { | ||
| int max = 0; | ||
| for (Car car : cars) | ||
| { | ||
| if (car.getPosition() > max) | ||
| { | ||
| max = car.getPosition(); | ||
| } |
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.
저는 개인적으로 이런 형식의 코드를 쓸때 Stream을 사용하는것을 선호하는데, 준영님의 의견은 어떤지 궁금합니다.
|
커밋 prefix에 대해서도 알아보시면 좋을거같아요😊 |
감사합니다.