[엘리] Onboarding 미션 제출합니다.

[YebinK]

마감시간을 지키느라 급하게 마무리해서 미완성인 점이 좀 아쉽습니다.
다음 미션 때는 좀 더 시간을 넉넉히 두고 임해야 할 것 같습니다.

[Hue9010]

이번 리뷰를 맡은 휴라고 합니다. :)
전체적으로 요구사항에 맞게 잘 작성해주셨네요. 👍
몇가지 피드백 남겼습니다. 잘 부탁드립니다~!!

[Hue9010]

ONE을 그대로 1로 썼을때랑 큰차이가 있을까요?
매직 넘버를 기호상수로 변경하는 건 숫자 값이 무엇을 의미하는지 알기 힘들기 때문에 그 의미를 바로 알려주기 위한 성격이 강한데 숫자 그대로의 의미만 가진 현재의 ONE은 큰 의미가 없고 도리어 불필요한 변수를 생성한게 될 수도 있습니다.

마찬가지로 operatorIndex도 마찬가지로 보이네요.
Double.parseDouble(numbers.get(ZERO)) 같은 경우는 의미를 뽑자면 FIRST_RESULT로 뺼 수 있을 것 같네요. 이렇게 되면 오퍼레이터들의 첫번째 인덱스인 0과는 같은 값이지만 다른 의미로 쓰였기 때문에 기존 ZERO와 별개로 추가하여 사용하시거나 기존 ZERO의 사용 여부를 좀더 고민해 보셔야 할 것 같습니다.

[YebinK]

private final static int FIRST_RESULT = 0;

for (int operatorIndex = FIRST_RESULT, numberIndex = 1; operatorIndex < operators.size() ; operatorIndex++, numberIndex++)  { ~ }

이렇게 FIRST_RESULT와 for문을 사용해서 피드백 주신대로 고쳐보았습니다! 감사합니다! 한가지만 여쭤봐도 될까요? numberIndex를 초기화할 경우, FIRST_RESULT + 1로 하는 게 나을지, 그냥 1로 하는 게 나을지 의문입니다. 같은 배열의 인덱스를 참조하기에 FIRST_RESULT를 넣어 통일감을 주고싶은데, 너무 코드가 길어지는 것 같아 의문이 듭니다.

[Hue9010]

사실 FIRST_RESULT도 한글로 직역하면 첫번째 결과라 Number가 첫번째 결과를 가지고 있는게 맞는지에 따라서는 변경의 여지가 큰 명칭이긴 합니다. 프로그래머가 가장 많은 시간을 소비하는 작업 중 하나가 변수명 짓기라는게 괜히 나온게 아닙니다. 😭

대신 numbers.getFirst(), numbers.getFirstResult()도 가능하겠네요.

원론적인 얘기를 먼저 드리자면 통일성 보다는 의미를 잘 전달하는지, 가독성이 어떤지(명확한 의미를 전달한다면 길이 신경 X)에 집중하는게 좋답니다.

첫번째 결과로 두고 다시 고민해보자면 numbers에 0번 인덱스에 5가 들어있는 상황에서 첫번째 결과 + 1의 의미는 6이 맞을까요, 1이 맞을까요? 지금 의미에서는 FIRST_RESULT + 1의 사용은 바람직하지 못한 것 같네요. 지금은 numbers.get(FIRST_RESULT)로서 get()과 함께 첫번째 결과를 달라는 의미로도 일부 해석할 여지가 있어 의미가 있다고 봅니다.

마찬가지로 int operatorIndex = FIRST_RESULT도 0의 값의 재사용 측면이 강하다고 보내요. 반복문 시작 시 0이 첫번 째 인덱스를 의미하는 건 누구나 아는 사항이라 굳이 피하실 필요가 없답니다. 도리어 다른 사람 코드에서 0이 아닌 int operatorIndex = FIRST_RESULT로 되어 있다면 저 값은 무엇일까 고민하게 될 확률이 높답니다.

[YebinK]

아 그렇군요! 답변 감사합니다!
그럼 새로운 변수를 생성할 필요 없이 int operatorIndex = 0 이렇게 쓰는게 깔끔하겠네요!
감사합니다!

[Hue9010]

https://www.geeksforgeeks.org/loop-java-important-points/
알고 계실 수도 있겠지만 참고로 안내해 드리면 위 링크에 두번째 있는 Initializing multiple variables를 보시면 for문에 여러 값을 초기화 하여 사용할 수 있답니다. 양 쪽다 정답이 있는건 아니기 때문에 참고만 하시기 바랍니다 :)

[Hue9010]

enum 사용 👍
다만 전체적으로 if문 사용이 많아보이는데 이후 나머지를 구하는 % 등 다른 연산들이 추가되면 매번 if문 까지 손봐야하는 번거로움이 생기게 됩니다. PLUS, MINUS, ..에 따라서 다형성을 가지게 하면 어떨까요?

고민해볼 여지를 줄여버리긴 하지만.. 아래 링크에 좋은 예제들이 있답니다.
https://www.slipp.net/questions/566

[Hue9010]

사용하지 않는다면 제거!
여기서 잠깐 질문을 드리자면 Calculator의 run이 정말 필요할 까요?
run에 있는 항목을 전부 CalculatorApplication의 main으로 옮겨도 Calculator의 역할에 변동이 있을까요?

[Hue9010]

Suggested change

	if (input.replace(" ", "").length() == 0) {
	if (input.replace(" ", "").isEmpty()) {

도 가능하답니다! 👍
구현 내용을 확인해보셔도 실제로 똑같이 length == 0 을 확인하는 걸 알 수 있답니다! 😄

[Hue9010]

        List<OperatorGroup> expected = new ArrayList<>();
        expected.add(OperatorGroup.PLUS);
        expected.add(OperatorGroup.MULTIPLY);
        assertThat(result).isEqualTo(expected);

이 되어야 할 것 같네요. 변경이 있을 때마다 가능하면 전체 테스트 코드를 최소한 변경이 일어난 부분에 해당하는 테스트를 돌려 보는게 좋답니다! 👍

[YebinK]

아 그렇네요! 네 다음 미션부터는 즉각 테스트코드에 반영해보도록 하겠습니다!

[Hue9010]

너무 많은 내용이 들어 가고, 과정을 진행하면서 배우실 내용이라 조금만 설명 드리자면 지금 형태에선 원래 의도 했던 final 키워드 그대로 처음 values를 초기화 한 이후 변경이 불가능함을 보장하지 못합니다. 실제로도 빈 리스트를 할당 한 이후 리스트 내부의 값이 추가되고 있죠.

values = Collections.unmodifiableList(new ArrayList<>(inputs));

그래서 위 코드처럼 변경이 불가능한 리스트로 처음 초기화 해줘야 합니다. 외부에서 받아온 inputs을 그대로 사용해 버리면 Numbers의 values는 변경이 불가능 하지만 외부에서 inputs에 값을 추가하거나 제거할 가능성이 남아있답니다.

지금 단계에서 이걸 너무 크게 고민하시기보단 아직은 이런게 있다 정도로만 넘어가셔도 나중에 비슷한 상황이나 학습 시에 깨닫게 될 겁니다! 👍

이 부분에 대해서 추가적으로 궁금하시면 DM이나 코멘트 남겨주시면 적당한 자료를 찾아서 남겨드리도록 할게요. (지금 자료 접근이 제한적이라 적절한 수준에 좋은 자료를 못 찾겠네요 😢 )

[Hue9010]

stream을 반복문으로만 사용한다면 아래 코드와 가독성 측면에서 큰 이점이 없어 좋지 않답니다.
다만 stream 연습 차원에서 하셨다면 👍

        for (String number : numbers) {
            values.add(number);
        }

하드웨어 성능도 좋아지고 정말 성능 이슈가 큰 부분이 아니라면 성능을 크게 고려 안해도 된다고 생각하지만, 가독성 차원에서도 큰 이점이 없다면 지양하는게 좋답니다.

[Hue9010]

사실 FIRST_RESULT도 한글로 직역하면 첫번째 결과라 Number가 첫번째 결과를 가지고 있는게 맞는지에 따라서는 변경의 여지가 큰 명칭이긴 합니다. 프로그래머가 가장 많은 시간을 소비하는 작업 중 하나가 변수명 짓기라는게 괜히 나온게 아닙니다. 😭

대신 numbers.getFirst(), numbers.getFirstResult()도 가능하겠네요.

원론적인 얘기를 먼저 드리자면 통일성 보다는 의미를 잘 전달하는지, 가독성이 어떤지(명확한 의미를 전달한다면 길이 신경 X)에 집중하는게 좋답니다.

첫번째 결과로 두고 다시 고민해보자면 numbers에 0번 인덱스에 5가 들어있는 상황에서 첫번째 결과 + 1의 의미는 6이 맞을까요, 1이 맞을까요? 지금 의미에서는 FIRST_RESULT + 1의 사용은 바람직하지 못한 것 같네요. 지금은 numbers.get(FIRST_RESULT)로서 get()과 함께 첫번째 결과를 달라는 의미로도 일부 해석할 여지가 있어 의미가 있다고 봅니다.

마찬가지로 int operatorIndex = FIRST_RESULT도 0의 값의 재사용 측면이 강하다고 보내요. 반복문 시작 시 0이 첫번 째 인덱스를 의미하는 건 누구나 아는 사항이라 굳이 피하실 필요가 없답니다. 도리어 다른 사람 코드에서 0이 아닌 int operatorIndex = FIRST_RESULT로 되어 있다면 저 값은 무엇일까 고민하게 될 확률이 높답니다.

[Hue9010]

요구사항에 맞게 잘 작성해주셨네요. 👍 머지 했습니다!
몇가지 생각할 사항 남겨드렸는데 보시고 추가적으로 궁금하신 부분은 편하게 코멘트나 DM 보내주셔도 됩니다 :)
고생하셨습니다!