Skip to content

mission 12#282

Open
JunoLee1 wants to merge 7 commits intocodeit-bootcamp-nodejs:mainfrom
JunoLee1:clean-main
Open

mission 12#282
JunoLee1 wants to merge 7 commits intocodeit-bootcamp-nodejs:mainfrom
JunoLee1:clean-main

Conversation

@JunoLee1
Copy link
Collaborator

요구사항

기본

  • 선택 / 병합/ 삽입/ 퀵 정렬

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

Copy link
Collaborator

@rdd9223 rdd9223 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 구현 및 디테일에 다소 아쉬운 점이 있어요. 예외 케이스들도 통과할 수 있도록 다양한 값을 넣어서 시도해보시는 것을 추천드립니다.

}
const l_sorted = quickSort(leftNums);
const r_sorted = quickSort(rightNums);
return [...l_sorted, pivot, ...r_sorted];
Copy link
Collaborator

Choose a reason for hiding this comment

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

"해당 배열을 수정하도록" 구현이 되어있어야 하는데, 새 배열을 생성하고 있어요. nums 자체를 sort하도록 수정해보세요. (재귀)


*/
function selectSort(nums) {
n = nums.length;
Copy link
Collaborator

Choose a reason for hiding this comment

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

let / const 키워드를 누락해서 전역변수로 취급됩니다. 이경우는 다른 함수에 영향을 줄 수 있어서 옳지 못한 코드로 보여요. 꼭 let 혹은 const로 선언 후에 사용해주세요.

Comment on lines +210 to +211
leftIdx = 0;
rightIdx = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

선언만 하고 사용하지 않는 변수는 제거하는 것이 좋습니다.

=> O(n)

*/
function selectSort(nums) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

함수 명은 요구사항과 일치하도록 해주세요.

Comment on lines +223 to +224
const l_sorted = quickSort(leftNums);
const r_sorted = quickSort(rightNums);
Copy link
Collaborator

Choose a reason for hiding this comment

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

snake_case로 통일하거나, camelCase로 통일해주세요.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants