[자동차 경주 게임] 코즈 미션 제출합니다. #64
Conversation
comma로 나뉘어진 숫자의 합을 반환 하는 메소드 작성.
calculate 메소드에서 입력값의 구분자가 colon인 경우에 대한 테스트 추가 작성.
Positive 일급 객체 사용으로 유효성 검사를 진행.
null 또는 공백이 입력된 경우 0을 반환한다.
입력값이 숫자인지 판별한다.
입력값이 1이상인지 확인한다.
domain 및 util 패키지 생성.
domain 및 util 패키지 생성.
toPositives 생성.
List를 반환하는것이 아닌 Stream을 반환하도록 변경.
Record를 사용하여 게임 진행과 결과 출력을 분리합니다.
There was a problem hiding this comment.
이번 리뷰를 맡게된 휴라고 합니다 :)
각 객체에 맞게 책임 분리를 꼼꼼히 잘해 주셨네요!
전체적으로 final을 많이 사용하셨는데 불변성을 고려하여 작성 한거라면 불변 객체가 아닌 것들이 일부 존재하는데
한번 고민 해보시고 불볍 객체로 마저 완성해주시거나, 불변 객체로 처리 할 객체가 아니라고 생각되시면 final 키워드를 제거해주세요. (혹은 처리가 힘드시다면 제거만 해주시고 고민 된 부분 문의 주세요!)
final 키워드로 인해 다른 사람들이 불변 객체로 오해 할 수 있답니다.
그리고 Cars에 대한 테스트도 한번 마저 작성해보세요 😋
| public static void main(final String[] args) { | ||
| final Cars cars = CarsFactory.create(InputView.inputNames()); | ||
| final TryCount tryCount = new TryCount(InputView.inputTryCount()); | ||
| final Record record = new Record(); |
There was a problem hiding this comment.
final 키워드를 달아 불변할거라고 기대되지만 사실 불변한 객체가 아니랍니다.
아래 글을 보면 딱 Record와 같은 예제를 보여주네요 :)
http://wonwoo.ml/index.php/post/1173
| package tdd.racingcar.domain; | ||
|
|
||
| public class Car { | ||
| private final Position position = new Position(0); |
| import java.util.function.Consumer; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public class Cars { |
| } | ||
|
|
||
| public void move() { | ||
| cars.forEach(tryToMove()); |
There was a problem hiding this comment.
for (Car car : cars) {
final Power randomPower = PowerFactory.createRandomPower();
car.move(randomPower);
}전체적으로 람다를 활용하는 방향으로 Iterable의 forEach를 많이 사용하셨는데 향상된 for문도 한번 사용해보시는건 어떨까요?
학습을 하는 차원에서 람다를 많이 사용하는 것도 도움이 되고 좋게 봅니다.
다만 지금의 forEach와 Consumer를 조합하여 사용하는 방식이 보편적인 않은 환경이라면 서로간의 코드를 이해하는데 약간의 시간을 사용하게 되기도 하고, 충분히 지금 방식의 반복은 익숙하신 걸로 보여서 향상된 for문에 적응해 보는 것도 추천드립니다.
이미 잘 사용 하실 수도 있을 것 같네요 :)
| return position.match(maxPosition); | ||
| } | ||
|
|
||
| public int getPosition() { |
There was a problem hiding this comment.
일급 객체를 좀 더 철저하게 사용해서 int가 아니라 position을 반환하게 바꿔보세요.
한가지 힌트를 드리자면 출력부분에는 Position의 toString을 재정의해서 숫자가 그대로 출력되게 할 수 있답니다.
| return cars.stream() | ||
| .mapToInt(Car::getPosition) | ||
| .max() | ||
| .orElseThrow(() -> new IllegalArgumentException("차가 존재하지 않습니다.")); |
There was a problem hiding this comment.
IllegalArgumentException 는
부정한 인수, 또는 부적절한 인수를 메서드에 건네준 것을 나타내기 위해서 발생 됩니다.
현재 상황에서는 다른 Exception이 좋을 것 같습니다!
| return position.getPosition(); | ||
| } | ||
|
|
||
| public String getName() { |
| private final List<Car> cars; | ||
|
|
||
| public Cars(final List<Car> cars) { | ||
| this.cars = cars; |
There was a problem hiding this comment.
final 키워드가 있다고 cars가 의도한대로 불변할지 한번 생각해보세요!
|
안녕하세요! 코즈입니다. 먼저 좋은 피드백 감사드립니다 😊 현재, 최대한 final을 붙이는 연습과 람다식을 사용하는 연습을 하고 있습니다. 보내주신 피드백 덕분에 final에 대한 깊은 고민과 방어적 복사, 그리고 람다식만 쓰는것에 대한 부작용에 대해 생각해볼 수 있었습니다. 정말 감사드립니다. 궁금한 것은 재할당만 금지하는 방법이 있는지에 대한 부분입니다. 제가 최대한 final을 붙이게된 배경은 재할당은 버그가 발생하기 쉬운 부분이라고 학습 했던점과, 최대한 객체를 훼손시키지 않고 사용하는걸 연습하기 위함 이었습니다. 때문에, 제가 final Record record 또는 final Position과 같이 선언하였던 것은, 객체의 상태는 변경되겠지만 최소한 재할당은 금지한다는 의미였다고 생각합니다. 이번 리뷰를 통해 final을 객체 또는 컬렉션에 선언할 경우, '해당 객체는 불변 객체이다' 라는 것을 암시하게 된다는 것은 새로 알게된 지식입니다. 하지만, 이런 지식과 맞물려 생각이든건 '그렇다면 재 할당만 금지하는 수단이 있는가?' 였습니다. 혹시, 이러한 고민들에 대해서 제가 잘못된 생각을 하고 있는 부분이 있거나, 관련된 지식을 알려주신다면 정말 감사드릴 것 같습니다..! |
|
추가적으로 혹시, Position의 move, Record의 append 같은 메소드가 내부 변수를 변경하는 것이 아닌, |
|
약간 잘못된 표현을 했었네요. final이 있으면 불변 객체로 생각하기보다는 해당 변수가 변하지 않을 것이라는 걸 기대하게 된다가 맞겠네요. final이 기능상으로는 한번의 초기화 이후 재할당이 불가능한게 끝이지만 그 결과 final 변수는 변하지 않을 것이라는 기대가 따라오게 됩니다.
아쉽게도 재할당만을 금지하기 위한 방법이 final의 기능이기 때문에 final을 사용하는게 맞습니다. 거꾸로 불변/가변 객체임을 나타낼 방법이 없다가 맞을 것 같네요. 솔직히 말씀드리자면 저는 final을 별로 사용하지 않습니다. 그렇다고 코즈님이 잘못 생각하고 있다고 생각하지도 않습니다. 이 부분은 개인 혹은 팀의 컨벤션 혹은 자신이 담당한 도메인 성격에 따라 다를 수 있는 부분이라고 생각합니다. 아무래도 final을 사용하지 않는 이유가 궁금하실 수도 있을텐데 딱 공감을 하는 부분이라서 책의 내용을 옮기자면
저같은 경우는 테스트로 다 잡아 내고 있다고 하진 못하겠고... 😢 대신 필요한 곳에 필요한 변수만 만들어 사용하여 알 수 없는 곳에서 다시 재할당 하는 위험성 자체를 줄입니다. 그리고 람다식 부분에도 말씀드렸다시피 학습 차원의 사용이라는 의도도 충분히 final, 람다 등 무엇이 됐던 그것을 사용하는 충분한 이유가됩니다. 특히나 적절한 상황에 적절한 걸 사용하는게 좋다는데 그것도 많이 사용해봐야지 알수 있는 부분이기 때문에 도리어 적극 사용해 보시는 걸 추천 드립니다. 다만 코즈가 사용하시는 걸 보이 이미 어느정도 익숙하게 사용하시는 걸로 보여 그 다음 단계로 정말 이게 맞는지 생각해보시길 바래서 피드백을 드렸답니다. 😄
값이, 상태가 가변한다는게 무조건 나쁜건 아닙니다. 바뀔건 바뀌어야죠. 다만 바뀌지 말아야 할 것이 바뀌거나 바뀔 수도 있다는 위험성을 가진게 문제일 뿐입니다. 사실 지금 final 키워드에 대해서 한번 생각해보고, 스스로가 final을 어떻게 사용하고 있는지 확인하는 정도도 충분하다고 생각합니다. 특히나 아직 2월이기 때문에 지속적으로 사용해보면서 고민해보시는 걸 추천드리긴 합니다. |
|
일급 컬렉션 (First Class Collection)의 소개와 써야할 이유 조금은 다른 글이지만 이 링크의 |
|
정성스러운 피드백 정말로 감사드립니다. 제가 궁금해하였던 부분들 명쾌하게 해결되었습니다. 재할당에 대해선 말씀해주신 테스트로 검증하며 final을 제거하여 가독성을 높인다는 것과, 최대한 필요한 곳에서 필요한 변수만 만들어 위험성을 줄이는 것 모두 좋은 지향점이라고 생각이 됩니다. 피드백 덕분에 제가 어떤 의지를 담아 코드를 작성하고 있는지 좀 더 명확하게 알 수 있는 계기가 된 것 같습니다. 현 시점에서 느끼는건 불변 객체를 표현할 수 없는 자바에서 final을 전부 붙혀주기 보단 재할당과 같은 문제를 의식적으로 처리하고, 불변 객체에 final을 붙혀 표현하는 것이 여러모로 경제적인 방법이라고 생각이 됩니다. Records의 StringBuilder를 사용했던 부분은 사실, 맨 처음엔 큐로 작성을 하였습니다. 보내주신 피드백들 반영하여 리뷰 요청드렸습니다! |
| package tdd.racingcar.domain; | ||
|
|
||
| public class Records { | ||
| private String records; |
There was a problem hiding this comment.
https://techcourse.woowahan.com/s/wTqesI5s/ls/GXDJNdI9
Record의 append 같은 메소드가 내부 변수를 변경하는 것이 아닌, 변경된 상태의 객체를 반환 이 부분은 StringBuilder가 매번 문자열을 생성하지 않기 위해서 사용하는데 매번 생성을 하면 결국 String이랑 다를 점이 없다는 것을 말씀 드리고 싶었던 건데 너무 많은 여지를 열어 드렸었네요. 🤣
|
슬랙 채널로 보내지기엔 내용이 길어질 부분이 있어 추가적으로 한가지 더 말씀드리면 기술적으로는 이미 잘 구현하시는지라 왜 혹은 필요한가를 한번 생각 해보시는 것도 많은 도움이 되실 겁니다. 그렇다고 지금 처럼 작성하시는 것에 대해서는 방어적인 자세가 되진 않았으면 좋겠네요. 가능하면 마음껏 사용해보면서 정말 필요한지 혹은 왜 써야하는지 생각해보는 과정이 추가되면 좋을 것 같아요. 추후에 면접을 보게 되는데 그런 부분들이 있다면(예를 들어 이번의 final) 정말 알고 쓴건지 좋다고 하니까 그냥 쓴건지 질문이 들어 올 수 있답니다. 이전 피드백에도 말씀드렸다시피 팀이나 개인에 따라서는 final을 정말 신경 써야 할 정도로 중요한 부분이 존재할 수 있다고 생각합니다. 제가 그러한 파트는 아니라서 어림짐작만 하자면 정산 등 돈에 대한 복잡한 계산이 들어가는 부분이 있다면 바뀌지 않아야 값이 실수로 바뀐다면 큰일이 나겠죠? 근데 그러한 값들이 많이 존재한다면? 아마 저라도 final을 적극 사용할 겁니다.(제가 겪어보진 않아서 |
|
마지막까지 감사드립니다.. 말씀해주신 말들 여러번 읽어보며, 정말로 지금 시기에 저에게 중요한 말이었음을 느낍니다. 자바나 객체 지향에 경험이 없어, 학습을 시작하면서 부터 좋은 코드는 무엇인지, 어떠한 부분에서 명쾌한 추상화라고 느끼는지, 그 감각을 익히고 흉내내려고 노력했습니다. 그 결과 어느 정도는 코드 스멜도 느끼고 안티 패턴엔 어떤것들이 있는지 알게 된것 같습니다. 오히려 제가 가진 문제점은 코드 스타일이 비슷하거나 수준이 높은 사람보단, 그렇지 않은 사람을 페어로 만나게 되었을때 마주 칠지도 모르겠습니다. 앞으로 겉으로 화려하기 보단, 좋다는 것들에 무 비판적인 수용을 지양하고, 여러 가지 시도들 해보며 왜 이게 좋은 코드인지, 나는 어떤 의도로 이 코드를 작성 하고 있는지 설명할 수 있는 사람이 되기위해 노력하겠습니다. |
No description provided.