Skip to content

Conversation

@le2sky
Copy link
Contributor

@le2sky le2sky commented Apr 26, 2022

대진표가 부전승이 없는 특성을 이용해서 풀이했습니다. 리뷰 부탁드립니다

@prove-ability
Copy link
Contributor

prove-ability commented Apr 26, 2022

제 PR 인줄 알고 close 누르고 reopen 했습니다..!!
그리고 Assignees, Labels 수정했습니다!

Copy link
Owner

@codeisneverodd codeisneverodd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 코드 잘 보았습니다 😁
함수를 적절히 분리해주신 점이 너무 좋았습니다!!
꼭 개선해주셨으면 하는 부분은 없어 확인후 Merge 진행합니다!

Comment on lines +34 to +38
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌱 반영해도 좋고 넘어가도 좋아요. 🌱

위의 다른 코드들 처럼 ternary operator를 사용해보시는 것도 좋을 것 같습니다!

Suggested change
if (isLeft()) {
arr.splice(arr.length / 2)
} else {
arr.splice(0, arr.length / 2)
}
isLeft() ? arr.splice(arr.length / 2) : arr.splice(0, arr.length / 2)
Comment on lines +24 to +26
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌱 반영해도 좋고 넘어가도 좋아요. 🌱

불필요한 tenary operator가 사용되어 가독성이 조금 떨어진다고 생각합니다! boolean 값을 직접 리턴 하시는 것보단 비교문 그 자체를 쓰시는 것이 좋을 것이라 생각됩니다!

Suggested change
return (
(arr.indexOf("A") + 1 > arr.length / 2 && arr.indexOf("B") + 1 <= arr.length / 2) ||
(arr.indexOf("A") + 1 <= arr.length / 2 && arr.indexOf("B") + 1 > arr.length / 2)) ? true : false
return (
(arr.indexOf("A") + 1 > arr.length / 2 && arr.indexOf("B") + 1 <= arr.length / 2) ||
(arr.indexOf("A") + 1 <= arr.length / 2 && arr.indexOf("B") + 1 > arr.length / 2))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌱 반영해도 좋고 넘어가도 좋아요. 🌱

이전 코멘트와 동일하게 불필요한 boolean 값들을 제거하시면 더 좋을 것이라 생각합니다!

Suggested change
return (arr.indexOf("A") + 1 > arr.length / 2) ? false : true
return arr.indexOf("A") + 1 <= arr.length / 2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 생각은 전혀 못했었네요 항상 좋은 리뷰 감사드립니다! ㅎㅎ

@codeisneverodd codeisneverodd merged commit 3fdce79 into codeisneverodd:main Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants