-
Notifications
You must be signed in to change notification settings - Fork 98
Update 풀이추가 #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update 풀이추가 #81
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RyanDeclan 님 기여해주셔서 감사합니다!!
친절하게 주석도 달아주셔서 너무 좋네요! �형식도 잘 맞추어주셨습니다!
감사한 마음을 담아 코드리뷰를 남겼으니 도움이 되셨으면 좋겠습니다 😄
다음에 기여하실 때 PR에 고민한 점이나, 궁금한 점을 적어주시면 적극 리뷰하겠습니다 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"반영해도 좋고 아니어도 좋아요"
solution 함수 내에서만 result가 사용되므로 이를 전역 변수로 선언하는 것은 권장되지 않습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"반영해도 좋고 아니어도 좋아요"
remainder가 0 즉 나누어 떨어지는 경우를 표현하신 것으로 보입니다! 해당 조건문도 잘 동작하지만 remainder === 0 과 같은 방식이 더 가독성이 좋을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"반영하면 좋을 것 같아요!"
replace 메서드 또한 병합할 수 있을 것 같습니다!
return result.replace(/["3"]/g,"4").split("").reverse().join("")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"반영하면 좋을 것 같아요!"
해당 조건문은 불필요해보입니다! quotient 의 값이 0이 아닌 경우 동작하는데 0인 경우에는 덧셈을 해도 무방하기 때문에, 해당 조건문을 그냥 문으로 고쳐도 좋을 것 같습니다! 불필요한 조건문은 가독성을 해치니까요!
result += quotient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"꼭 반영해주세요"
k=1 보단 let k = 1 이 더 적절해보입니다! 변수 선언 키워드를 지정하지 않은 k는 전역 변수로 선언되어 사이드 이펙트를 유발하기 쉽습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위의 k 의 경우와 동일합니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"의견"
complete[0] && completeBox.push(...complete) 와 같은 논리곱 연산자 사용도 고려해보세요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"반영해도 좋고 아니어도 좋아요"
의도하신 것이 아니라면 동등 비교 연산자는 ===를 사용하는 것이 권장됩니다! 자바스크립트는 암묵적인 타입변환이 쉽기 때문에 서로의 타입까지 비교하는 === 사용을 권장합니다.
No description provided.