[LBP] 김태우 자동차 미션 1-3단계 제출합니다. #92
Conversation
There was a problem hiding this comment.
안녕하세요 태우 👍
1~3단계 미션도 고생하셨습니다. 프로그래밍에는 정답이 없기 때문에 스스로 생각해보면 좋을 주제 위주로 답변드렸습니다. 근거를 기반으로 코드를 작성하고 피드백 받고 수정하는 사이클을 반복하다 보면 태우가 말씀하셨던 질문에 대해서도 스스로 답하실 수 있을 것 같아요. 😄
indent depth가 2를 넘지 않는 규칙을 준수하기 위해 람다 표현식과 메소드 참조를 많이 사용했으며, 이 부분이 어려웠습니다. 예혁님도 이것을 처음 접하셨을 때 어려움을 느끼셨을 것 같은데, 익숙하게 사용하기 위한 팁이 있을까요? 혹은 추천해 주실 만한 강의나 영상을 알려주실 수 있나요?
- 들여쓰기 2를 넘지 않는 규칙을 보셨을 때 그 이유에 대해 생각해보신 적이 있으신가요? 아니면 단순히 규칙이기 때문에 지키려고 하셨던 건가요. 태우의 생각이 궁금해요!
- 람다와 메서드 참조를 위해 공부한 적은 딱히 없는 것 같아요. 필요에 의해 사용해보고 경험하는 것이 가장 많이 도움될 것 같네요. 추가로 다양한 사람의 코드를 보는 것도 도움이 되는 것 같아요.
람다 표현식을 이용하기 전 Car 객체를 저장하는 컬렉션으로 2차원 배열, HashMap 등을 도입하려 노력했으나 끝내 indent depth 규칙을 극복하지 못했고 상당 부분을 람다 표현식으로 해결했습니다. 제 코드에서 도입해볼 수 있는 또다른 자료구조가 있을지 조언해주시면 감사할 것 같습니다.
- 하나의 클래스에서 책임지고 있는 일이 많은 것은 아닐까요? 각 클래스가 어디까지 책임져야 하는가에 대해서 생각해 보시고 적절하게 나누다 보면 들여쓰기 규칙도 준수하기 쉬워지는 것 같아요.
- 클래스를 적절히 분리하고, 메서드가 하나의 일만 수행하도록 분리하다 보면 자연스럽게 들여쓰기 규칙이 거의 지켜진다고 생각해요. 한번 시도해 보는 것은 어떨까요? 😄
메소드의 이름을 최대한 직관적으로 지으려고 노력했으나, 코드를 처음 본 사람이 즉각적으로 이해할 수 있을지 걱정이 많이 됩니다. 코드를 여러 번 읽다 보니 이 코드를 처음 본 사람의 입장에서 생각하기가 어려워지는 문제가 발생해 점점 메소드의 이름을 함부로 수정하기가 어려웠습니다. 메소드 이름을 짓는 팁 혹은 예혁님께서 사용하시는 메소드 작명 방법을 알려주실 수 있나요?
- 다양한 사람의 코드를 자주 보는 것이 많이 도움이 되었어요. 다른 사람의 코드를 이해하려고 하면, 반대로 어떻게 작성하면 이해하기 어려운지 알 수 있거든요. 그리고 팀원과의 약속, 컨벤션 등에 따라 달라지기 때문에 상황에 맞게 판단하고 작성해보고 피드백 받는 것을 반복하다 보면 더 나은 코드를 작성할 수 있을 것 같아요 👍
| public class Car { | ||
| int position = 0; | ||
| public int carNumber; | ||
| public String carName; |
There was a problem hiding this comment.
position 필드의 접근 제어자를 지정하지 않은 이유가 있나요?
There was a problem hiding this comment.
처음 선언할 때 코드의 안정성보다는 로직의 구현에 집중하다 보니 그냥 int position = 0; 으로 선언한 뒤 깜빡하고 말았습니다. 부끄러운 실수이지만, 이번 기회에 제 안좋은 습관을 확실히 알게 되었습니다. 앞으로는 필드를 선언할 때 접근 지정자를 꼼꼼히 확인하도록 하겠습니다! 👀
| public Car() { | ||
| this.position = 0; | ||
| } | ||
|
|
||
| public Car(int carNumber, String carName) { | ||
| this.carNumber = carNumber; | ||
| this.carName = carName; | ||
| } |
There was a problem hiding this comment.
이미 Car 클래스의 position 필드는 0으로 초기화 되어 있는 것 같아요.
기본 생성자에서는 0으로 다시 할당하고, 아래의 생성자에서는 할당하지 않은 특별한 이유가 있나요?
There was a problem hiding this comment.
상단의 접근 제어자를 지정하지 않은 것과 비슷하게, 코드 작성 시 코드의 실행 가능 여부에 집중했습니다. 그러다 보니 우선 코드가 작동만 되도록 작성한 뒤 미처 확인하지 못하고 커밋을 진행했습니다.
상단의 코맨트와 더불어 저의 안좋은 습관을 다시금 돌이켜 보게 되었습니다. 앞으로는 더 꼼꼼히 확인하도록 하겠습니다!
| public int getRandom() { | ||
| double randomValue = Math.random(); | ||
| return (int) (randomValue * 9); | ||
| } |
There was a problem hiding this comment.
- 자동차가 임의의 정수를 반환하는 책임을 가져야 하는 이유가 무엇일까요?
- 이대로 사용하신 예정이라면 메서드명을 더욱 명확히 할 수 있을 것 같아요. (랜덤한 정수라는 의미를 포함하거나 랜덤한 정수의 범위까지 메서드명에 추가하는 건 어떨까요?)
- 더 나아가 start와 end를 인자로 받아서 start부터 end 사이의 무작위 정수를 반환하도록 하면 어떨까요?
(예:getRandomNumberInRange(1, 9);)
- 더 나아가 start와 end를 인자로 받아서 start부터 end 사이의 무작위 정수를 반환하도록 하면 어떨까요?
- 추가로 테스트를 진행하기 어려운 형태일 것 같은데 태우님의 생각은 어떠신가요? (코드 작성 근거와 함께 설명해 주시면 좋을 것 같아요!)
There was a problem hiding this comment.
- 자동차가 임의의 정수를 반환하는 책임을 가져야 하는 이유가 무엇일까요?
코맨트를 읽고 다시 생각해 보니, 자동차는 임의의 정수를 반환하는 책임을 질 필요가 없다고 생각됩니다. 코드를 작성할 때 자동차가 임의의 정수를 반환하도록 한 이유는 두가지 입니다.
- CarRace에서 난수를 생성하면 불필요한 필드 변수가 필요할 것이라고 생각했습니다. 이제는 그럴 필요가 없다는 것을 알게 되었습니다.
- 자동차가 움직일 수 있는지 여부를 확이나는 getStatus 메소드가 Car 클래스 안에 있다 보니, 항상 같이 사용되는 getRandom() 메소드도 Car 안에 있는 것이 좋아 보였습니다.
결론적으로, 자연스럽게 좋은 코드가 아닌 내가 생각하기 편한 코드를 작성한 것 같습니다.
이대로 사용하신 예정이라면 메서드명을 더욱 명확히 할 수 있을 것 같아요. (랜덤한 정수라는 의미를 포함하거나 랜덤한 정수의 범위까지 메서드명에 추가하는 건 어떨까요?)
- 더 나아가 start와 end를 인자로 받아서 start부터 end 사이의 무작위 정수를 반환하도록 하면 어떨까요?
말씀하신 대로 코드를 리팩토링하여 CarRace 클래스에 추가해 보도록 하겠습니다!
(예:getRandomNumberInRange(1, 9);)
- 추가로 테스트를 진행하기 어려운 형태일 것 같은데 태우님의 생각은 어떠신가요? (코드 작성 근거와 함께 설명해 주시면 좋을 것 같아요!)
Car 클래스 안에 getRandom() 과 getStatus() 가 같이 있는 것이 더 통제하기 좋다고 생각해 이렇게 코드를 작성했습니다. 하지만 예혁님의 코맨트를 보고 테스트를 하는 관점에서 생각해 보니 이와 같은 구조는 getRandom()을 외부에서 조작할 수 없어 오히려 테스트를 진행하기 어려운 구조라는 것을 알게 되었습니다. 이 생각을 기반으로 코드를 리팩토링 해 보도록 하겠습니다.
P.S 코드 작성 근거와 함께 설명해 주시면 좋을 것 같다는 문장이 저로 하여금 정말 많은 고민을 하게 만들어 주었습니다. 또한, 제가 코드를 작성함에 있어 정말 근거가 부족한 상태로, 코드를 완전히 이해하지 못한 상태로 작성한다는 것을 다시금 깨닫게 되었습니다. 이 문장이 저에게 정말 많은 도움이 되었다고 생각합니다. 감사합니다. ❤️🔥
| public boolean getStatus() { | ||
| int randomValue = getRandom(); | ||
| boolean result = true; | ||
|
|
||
| if (randomValue <= 3) { | ||
| result = false; | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
- getStatus()라는 메서드명이 추상적이라 이해하기 어려웠던 것 같아요. 더 명확하게 작성해 보는 것은 어떨까요?
- 또한, 랜덤과 마찬가지로 해당 기능이 자동차라는 객체가 가져야 할 책임이라고 생각하신 이유가 궁금해요!
- result라는 변수 없이도 작성해볼 수 있을 것 같아요.
// 형태 예시
if (value > 4) {
return true;
}
return false;There was a problem hiding this comment.
-
아래 남겨주신 코멘트를 참고해 isMovalbe() 로 바꿔보도록 하겠습니다!
-
현재 해당 자동차의 상태를 나타내는 메소드이기 때문에 자동차라는 객체가 가져야 할 책임이라고 생각했습니다. 하지만 다시 생각해 보니, 즉석으로 만들어진 랜덤 값을 가지고 자동차의 전진 가능 여부를 판단하는 메소드는 굳이 자동차라는 객체가 가져야 할 책임이라는 생각이 드네요.
-
메소드는 더 간결하게 수정했습니다!
| public void tryMove() { | ||
| if(getStatus()) { | ||
| this.position++; | ||
| } | ||
| } |
There was a problem hiding this comment.
- 전체 메서드를 보기 전까지는 무슨 일을 하는지 쉽게 파악하기 힘든 것 같아요.
- 자동차 객체가 스스로 움직임을 시도하는 것보다. 그냥 움직이게 만들어 보는 것은 어떨까요?
- 자동차의 책임은 무엇일까에 대해 고민해보시면 좋을 것 같네요.
- 추가적으로 if문과 조건문 사이에 공백을 추가해주시면 좋을 것 같아요. 공백도 컨벤션입니다!
// 책임 분리 예시
// 자동차 외부 어딘가에서
if (isMoveable()) { // 움직일 수 있나? (랜덤 또는 새로운 움직임 규칙이 생겼을 때 대체 가능)
car.moveForword(); // 차량 앞으로 전진, (position++;)
}There was a problem hiding this comment.
-
제시해주신 방법이 정말 좋은 것 같습니다. 코드를 전체적으로 처음부터 재설계할 예정입니다. 재설계 과정에서 반영해 보도록 하겠습니다!
-
이렇게 메소드를 하나씩 뜯어보니 객체 하나와 그 안의 메소드를 만들 때, 제가 깊은 고민을 하지 않고 만들었다는 것을 알게 되었습니다. 자동차라는 객체는 자동차 자체의 상태와 정보를 저장하는 책임을 가져야 한다고 생각합니다.
-
또 막상 코딩을 할 땐 인지하지 못했지만, 다시 제 코드를 읽어보니 들여쓰기 2의 제한을 회피하기 위한 수단으로써 Car 객체가 가질 필요 없는 책임을 너무 많이 지웠다는 것을 인지하게 되었습니다. 상단의 내용들을 참고하여 코드를 전체적으로 리팩토링 해보도록 하겠습니다.
-
남겨주신 코맨트를 보고 다시 꼼꼼히 확인해 보니 공백 컨벤션이 일정하게 되어있지 않은 것을 발견했습니다. 앞으로는 더 꼼꼼히 확인하도록 하겠습니다!
| public void makeCarObject(int sizeOfList) { | ||
| for(int i = 0; i < sizeOfList; i++) { | ||
| cars.add(new Car(i, carNames.get(i))); | ||
| } | ||
| } |
There was a problem hiding this comment.
메서드명은 자동차 객체를 만드는 것으로 보이지만, 사실을 자동차 객체를 만들고 자동차 목록에 추가하는 일까지 수행하고 있네요. 어떻게 하면 좋을까요?
There was a problem hiding this comment.
자동차 객체를 만드는 것과 자동차 목록에 추가하는 역할을 분리할까 고민해 보았습니다. 하지만 메소드명을 makeCarList 로 바꾸는 것이 차라리 나을 것 같아 수정했습니다!
There was a problem hiding this comment.
- 자동차들을 List가 아닌 다른 자료구조를 사용하려고 한다면, 메서드명도 수정해야 할까요?
- make라고 하지만, 사실은 이미 만들어진 List 객체에 add() 메서드를 사용하여 Car 객체를 만들어 추가하고 있어요. 정말 Car List를 만드는 것일까요?
| public void setWinnerCarNames() { | ||
| int maxPosition = cars.stream() | ||
| .max(Comparator.comparingInt(Car::getPosition)) | ||
| .orElseThrow(NoSuchElementException::new) | ||
| .getPosition(); | ||
|
|
||
| winnerCarNames = cars.stream() | ||
| .filter(c -> c.getPosition() == maxPosition) | ||
| .map(Car::getCarName) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
이건 바로 수정하지 않고 스스로 생각해 보면 좋을 것 같아요. (Stream을 쓰는 것에 초점을 맞춘 것이 아닙니다!)
단순히 저의 의견이기 때문에 답장하지 않더라도 스스로 생각해보면 좋을 것 같아요. 객체의 역할과 책임에 대해서요.
현재 우승한 자동차의 이름(들)을 위해 filter 부분에서 자동차의 포지션을 getter로 얻은 후 비교하고 있어요.
자동차는 자신의 위치를 알고 있습니다. 자동차에게 다른 자동차를 주고 스스로 판단하게 만드는 것은 어떨까요? 즉, 객체에게 메시지를 주고 객체가 스스로 일을 하게 만드는 방향으로 작성해 보는 것도 좋은 경험이 될 것 같아요.
getter로 빼서 비교하거나 수정, 추가 등의 작업을 한다면, 해당 객체는 무슨 책임을 가지고 있는 것일까요? 단순히 데이터(상태)를 저장하는 것만이 중요할까요?
| public static void main(String[] args) { | ||
| CarRace cr = new CarRace(); | ||
|
|
||
| cr.receiveCarNamesInput(); | ||
| cr.recieveCarRaceAttemptCountInput(); | ||
| cr.makeCarObject(cr.carNames.size()); | ||
| cr.doRace(cr.raceAttemptCount); | ||
| cr.setWinnerCarNames(); | ||
| cr.printWinnerCarNames(); | ||
| } |
There was a problem hiding this comment.
cr로 축약하신 특별한 이유가 있나요? 오히려 저는 보기 어려웠던 것 같아요.
There was a problem hiding this comment.
CarRace 즉, 자동차 경주 객체가 사용자에게 입력을 받는 것을 책임져야 하나요?
There was a problem hiding this comment.
-
인스턴스 생성시 클래스명을 파스칼 케이스로 언급하다 보니 대문자로 표현되는 단어를 축약하는 것이 가독성에 좋을 것이라고 추측했습니다. 다시 생각해 보니 코드가 길어진다면 축약된 'cr'이 무슨 의미인지 알기 어려울 것 같네요. race로 수정해 보겠습니다!
-
다시 생각해 보니 CarRace는 사용자의 입력을 받는 책임을 질 필요가 없다고 생각되네요.. 단순히 main 메소드는 최대한 깔끔하고 가독성이 좋아야 한다고 생각하고 코드를 짰던 것 같습니다. 기본적인 부분들을 많이 배워가는 것 같습니다. 감사합니다!
| @Test | ||
| @DisplayName("랜덤 값에 따라 true, false 반환하고 doesNotThrowAnyException 예외 발생 여부를 확인한다.") | ||
| void shouldReturnStatusOfCarRaceTest() { | ||
| System.out.println("3이하 false, 4이상 true : " + car.getStatus()); | ||
| assertThatCode(car::getStatus).doesNotThrowAnyException(); | ||
| } |
There was a problem hiding this comment.
자동차 움직임이 랜덤에 의존되기 때문에 코드를 직접 제어할 수 없고, 그로 인해 이러한 코드가 만들어진 것 같아요. 자동차 경주와 랜덤 테스트에 대한 블로그 포스팅이 많으니 여러 개를 읽어보고 개선 방법을 찾아보시면 좋을 것 같아요.
There was a problem hiding this comment.
상단의 코맨트를 읽어 내려온 뒤 제가 짠 테스트 코드를 보니 정말 처참하기 그지없네요..
블로그 포스팅을 많이 찾아보고 참고하여 코드를 수정해 보도록 하겠습니다!!
| @Test | ||
| @DisplayName("자동차 경주 결과를 반환하고 doesNotThrowAnyException 예외 발생 여부를 확인한다.") | ||
| void shouldReturnResultOfCarRaceTest() { | ||
| cr.carNames = new ArrayList<>(Arrays.asList("neo", "brie", "brown")); | ||
| cr.raceAttemptCount = 5; | ||
| cr.makeCarObject(cr.carNames.size()); | ||
| cr.doRace(cr.raceAttemptCount); | ||
| assertThatCode(cr::setWinnerCarNames).doesNotThrowAnyException(); | ||
| } | ||
| } |
There was a problem hiding this comment.
@DisplayName에 doesNotThrowAnyException 예외 발생 여부를 확인한다. 보다는 정상적으로 자동차 경주가 진행되어 예외가 발생하지 않는다. 라는 형태는 어떨까요? 테스트 코드 도구의 결과를 표현하는 것보다는 어떤 경우에 예외가 발생하거나 하지 않거나 등을 작성하면 더욱 쉽게 이해할 수 있을 것 같아요.
There was a problem hiding this comment.
코맨트를 읽고 다시 생각해 보니, 규모가 큰 프로그램을 짜보지 못해서 생긴 문제라는 생각이 들었습니다. DisplayName에 적는 설명은 어느정도의 추상화가 필요한 것 같네요. 앞으로 DisplayName 작성 시 참고하도록 하겠습니다. 감사합니다!
|
|
|
스터디 정보
질문사항
indent depth가 2를 넘지 않는 규칙을 준수하기 위해
람다 표현식과메소드 참조를 많이 사용했으며, 이 부분이 어려웠습니다. 예혁님도 이것을 처음 접하셨을 때 어려움을 느끼셨을 것 같은데, 익숙하게 사용하기 위한 팁이 있을까요? 혹은 추천해 주실 만한 강의나 영상을 알려주실 수 있나요?람다 표현식을 이용하기 전 Car 객체를 저장하는 컬렉션으로
2차원 배열,HashMap등을 도입하려 노력했으나 끝내 indent depth 규칙을 극복하지 못했고 상당 부분을 람다 표현식으로 해결했습니다. 제 코드에서 도입해볼 수 있는 또다른 자료구조가 있을지 조언해주시면 감사할 것 같습니다.메소드의 이름을 최대한 직관적으로 지으려고 노력했으나, 코드를 처음 본 사람이 즉각적으로 이해할 수 있을지 걱정이 많이 됩니다. 코드를 여러 번 읽다 보니 이 코드를 처음 본 사람의 입장에서 생각하기가 어려워지는 문제가 발생해 점점 메소드의 이름을 함부로 수정하기가 어려웠습니다. 메소드 이름을 짓는 팁 혹은 예혁님께서 사용하시는 메소드 작명 방법을 알려주실 수 있나요?