Conversation
kigpand
added
Needs Review
| export default class Machine { | ||
| constructor(version) { | ||
| this.version = version; | ||
| this.machine = document.createElement("article"); |
There was a problem hiding this comment.
article 엘리먼트를 사용한 이유가 있을까요? this.machine 이라는 이름은 적절한 이름일까요?
코드를 그대로 해석하면 Machine 내부에 machine이 있다는 의미로 보입니다.
| this.machine = document.createElement("article"); | ||
| this.machine.className = `machine ${version}`; | ||
|
|
||
| document.querySelector("body").appendChild(this.machine); |
There was a problem hiding this comment.
document.body를 사용하지 않은 이유가 있을까요?
| document.querySelector("body").appendChild(this.machine); | ||
| } | ||
|
|
||
| on() { |
There was a problem hiding this comment.
머신이 실행된다는 의미로 메소드명을 지정했습니다
| behavior: "smooth", | ||
| }); | ||
|
|
||
| inputText.innerText = (inputNum - value).toLocaleString(); |
There was a problem hiding this comment.
toLocaleString은 자주 반복되는데, formatNumber 등으로 래핑하는 편이 더 직관적이지 않을까요? 또한 어떤 통화 체계에서는 세자리마다 컴마를 찍는 동작이 일어나지 않을 수도 있는데, 고려하신 걸까요?
There was a problem hiding this comment.
생각하지 못했습니다. 다시 확인해보겠습니다
| @@ -0,0 +1,9 @@ | |||
| export default class InputFieldButton { | |||
| constructor(version, label, price) { | |||
| this.element = document.createElement("button"); | |||
There was a problem hiding this comment.
이대로 동작은 하겠지만 element가 이 클래스의 프로퍼티라는 점을 명확히 하기 위해서 constructor 전에 선언해주면 좋을 것 같습니다.
| @@ -0,0 +1,9 @@ | |||
| export default class OutputFieldButton { | |||
There was a problem hiding this comment.
클래스를 사용해 OOP를 의도하신 거라면 element라는 공통 프로퍼티를 가지는 여러 클래스에 대해서 공통된 부모 클래스를 가져갔어도 되지 않을까요?
| outputTextArea = new OutputFieldTextArea(machine.element, version); | ||
| }); | ||
|
|
||
| it("투입 테스트", () => { |
There was a problem hiding this comment.
"투입 테스트"보다 더 상세하게 기술해주세요. 예를 들면 "투입한 금액만큼 표시되는 금액이 증가한다" 같은 식으로 해도 좋습니다.
2 participants
No description provided.