[2단계 - 행운의 로또 미션] 안(안수진) 미션 제출합니다.

[jin7969]

안녕하세요 로이님!! 😄
미션을 늦게 제출하게 되어 죄송합니다 ㅠ
처음에 모델과 뷰로만 구현을 하려고 하였으나, 코드가 복잡해지고 가독성이 좋지 않아, view에서 handlers를 생성하여 event 처리를 하였습니다.
데모 페이지

기능 요구 사항

• 결과 확인하기 버튼을 누르면 당첨 통계, 수익률을 모달로 확인할 수 있다.
  • 로또 당첨 금액은 고정되어 있는 것으로 가정한다.
• 다시 시작하기 버튼을 누르면 초기화 되서 다시 구매를 시작할 수 있다.

1단계 피드백 반영

• 테스트 코드 추가 작성
• private 설정
• comment [...Array(count)].map(this.generateLottoNumber) 으로 수정

리뷰 부탁드립니다!!

[roy-jung]

안님 고생하셨어요!
대대적인 수정을 가하려다 시간관계상 복구시키셨나보네요~ 실무에서도 그런 경험을 할 일이 생각보다 정말 많습니다. 좋은 경험 하셨네요 :)
1단계 피드백 반영하신 것 잘 확인했고, 2단계 내용도 전체적으로 잘 동작하네요.
코멘트 확인해 주시고, 아래에서는 UI/UX적으로 몇가지만 추가 언급하겠습니다.

• 로또를 20000원 정도 구매하고 번호보기를 누른 상태에서 결과모달을 출력하면, dimmed(뒷배경)이 전체화면을 덮지 않고 중간에만 위치합니다. 수정 필요해요!
• 입력한 값이 조건에 부합하지 않을 때, alert을 띄운 이후의 처리가 추가되면 좋겠어요. 예를 들어 문제가 발견된 input에 focus를 해준다거나, 어떤 부분이 오류인지를 색상 등으로 표시해준다거나..
• input에 값 입력후 enter를 누르는 것만으로 결과확인을 하거나 다음 input으로 넘어갈 수 있다면 더 좋겠습니다.

[roy-jung]

step1때 말씀을 안드렸긴 한데, 네이밍 관련해서 심각한 오류이니 반드시 수정해 주셨으면 합니다!

isValidNumberRange라는 명칭의 뉘앙스는, 숫자범위에 관련해서 valid한지, 즉 범위 안에 있는지를 묻는 느낌입니다. 개발자들 사이에선 isValid로 시작하는 함수의 결과는 조건 충족시 true, 그렇지 않을시 false라는 암묵적 합의가 이루어져 있어요. 그런데 안님의 실제 코드에서는, 범위 내에 있을 때 false, 범위 밖일 때 true를 반환하는 형태로, 컨센서스와는 정 반대로 동작하고 있습니다. 그러다보니 제삼자 입장에서는 혼란스럽고, 해당 함수를 찾아가서 코드를 찬찬히 분석한 다음에야 문제 없음을 이해하고도, 여전히 혼란스럽게 돼요.

비단 위 코드만이 아니라, 안님의 코드 중 isValid로 시작하는 모든 함수가 그렇습니다.

이 문제를 해결하는 두가지 방법이 있는데, 첫째는 네이밍만 반대로 바꾸는 것이고, 둘째는 네이밍은 그대로 둔 채 이 함수를 실제로 사용하는 코드들을 수정하는 방법입니다.

그 중 첫번째, 네이밍을 변경하는 방법은 간단하지만, 변수명 자체에 Not이 들어가게 되어 새로운 혼란을 야기하는 원인이 된다는 측면에서 바람직하지 않고,
두번째 방법의 경우 리팩터링을 위한 공수가 많이 들고 그러다보면 오류가 발생할 가능성이 있지만, 장기적으로는 더 나은 방향입니다.

[jin7969]

제가 잘못 알고 있었네요.. 감사합니다!!
다른 부분은 다 수정을 하였는데, isNotThousandUnit, isNotNumber 부분은
Not 을 "절대 사용하면 안된다"도 아니고, 구문상 부정문으로 하는게 낫다고 생각되어 사용을 하였습니다.

[roy-jung]

각 등수 및 당첨 갯수에 따른 수익률 계산이 정확한지를 좀 더 많은 테스트로 검증하는 편이 좋을 것 같습니다.

[jin7969]

잘 이해가 안되서요ㅜㅜ
1등 당첨시 수익률 테스트 1개, 2등 당첨시 수익률 테스트 1개... 이런식으로 각각 테스트를 만들어야 한다는 말씀이신가요?

[roy-jung]

네. 여러 등수 중 하나의 테스트가 통과되면 나머지 등수에 대한 수익률 계산도 모두 문제없이 되리라고 확신할 수 있나요? 테스트의 목적이 무언가요??

[jin7969]

저는 총 수익률 하나로 통합을 했었는데, 나머지 등수에 대한 확신은 할 수 없네요..
다시 수정하였습니다!!

[roy-jung]

보너스 매치 여부를 확인하는 로직은 matchNumberCount가 5인 경우에만 존재하도록 해도 충분하지 않을까요?
나머지 다른 케이스에 대해서는 위 코드를 실행할 필요가 없을 것 같아요.

[jin7969]

아 맞네요! 다른 케이스는 필요가 없군요
5인 경우에만 && 확인을 하도록 하였습니다.

    if (matchNumberCount === 5 && lotto.includes(winningNumbers[winningNumbers.length - 1])) {
      this.#lottoResult.second += 1;
      return;
    }

[roy-jung]

this.#method.bind(this) 는 this를 bind한 새로운 함수를 생성하는 명령입니다.
어차피 새로운 함수를 만드는건 똑같으니, 이보다는 처음부터 인스턴스 자신의 메소드로 들고 있게끔, class field로 선언해주면 더 좋다고 생각합니다.

constructor() {
  ...
  $('...').addEventListener('submit', this.#method);
}
#method = (e) => { ... }

[jin7969]

1. 화살표 함수의 this는 항상 인스턴스 객체를 가르킨다
2. 일반적인 메소드와 달리, 클래스 필드를 통해 정의한 메소드는 인스턴스를 생성할 때마다 새로 생성되기 때문에
   메모리를 더 차지 하게 되므로 주의를 해야 한다. 하지만, 여기서는 어차피 새로운 함수를 만드는건 똑같다!

이렇게 이해 했습니다!! 그런데.. 왜 이렇게 하는것이 좋은지는 잘 모르겠습니다. 이유가 있을까요??

[roy-jung]

1. 화살표함수의 this는 항상 인스턴스 객체를 가르킨다 -> 틀렸습니다. 단지 화살표 함수는 this를 바인딩하지 않을 뿐입니다. 마침 this가 인스턴스를 보는 컨텍스트에 있기 때문에 문제 없이 사용할 수 있을 뿐입니다.
2. '어차피 새로운 함수를 만드는건 똑같다' 맞습니다. 그런데 메소드선언방식에 의하면 prototype에 원본메소드가 남아있는 채로, 실제로 호출되는건 새로 만들어진 bound 메소드입니다. 반면 class field를 이용하면 prototype에는 남아있지 않고, 인스턴스 자신이 사용할 함수만 들고 있게 됩니다.

[roy-jung]

아래의 disableElement, enableElement 함수를 하나로 합치면 어떨까요?

[jin7969]

제 생각은 isDisable(true, false) 를 넘겨줘서 사용해 보았습니다.
로이님 생각은 어떠신가요?

// 클래스
resetPurchasedLotto() {
  this.#manageSwitchInput(true);
 ...
}

#manageSwitchInput(isDisable) {
  setElement(this.switchInput, isDisable);
}

// dom.js
export const setElement = (element, isDisable) => {
  isDisable ? element.setAttribute("disabled", true) : element.removeAttribute("disabled");
};

[roy-jung]

네 좋습니다 :)

[jin7969]

1단계 미션을 완료하고 설계를 어떻게 다시 해야 할지 막막하더라구요... 제 나름대로 이리저리 해보았지만, 잘 되지않아 어려움이 있었습니다.
하지만, 지금 생각해보면 로이님 말씀대로 좋은 경험을 한거 같아요. 😄

수정 사항

• 결과 모달창 뒷 배경이 전체를 덮도록 하였습니다.
• 당첨 번호가 잘못된 값 일 경우, js로 focus 줄려고 시도 하였으나, 제가 구현한 로직이 입력값 전체를 리스트로 넘겨줘서 유효성 검사를 하는 방식이라서 로직을 바꾸기 보단 html input 속성을 활용하여 잘못된 입력일 경우 알려주도록 하였습니다.
• 당첨 번호 태그를 form 태그로 감싸여 enter 입력시 결과 확인을 할 수 있도록 수정하였습니다.

다른 수정 사항은 위 코멘트에 작성하였습니다.

추가 사항

• 당첨 번호가 1~45 범위라서 2자리 숫자 입력시 자동으로 다음 번호를 입력할 수 있도록 변경 하였습니다.
• 구매 금액을 잘못 입력할 경우 input value를 초기화 하였습니다.
• 로또 게임을 다시 시작할때 구매한 로또 정보가 오류가 있어 수정하였습니다.

[roy-jung]

고생하셨습니다! 승인 및 머지할게요.
다음 미션도 화이팅이에요 :)
(추가로 몇가지 코멘트 남겼으니 확인해 보세요)