Skip to content

Conversation

@ljw0096
Copy link
Contributor

@ljw0096 ljw0096 commented Apr 27, 2022

구현 문제여서 각 단계마다 해야하는 작업을 function으로 분리해서 풀었습니다!

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.

좋은 코드 잘 보았습니다! 코드를 정말 깔끔하게 잘 짜시는군요👍👍
함수 분리가 잘 되어있어서 너무 좋았습니다 😆
다만 주석이 누락되어있어, Request Changes 상태로 두겠습니다!

Copy link
Owner

Choose a reason for hiding this comment

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

🌱 꼭 반영해주세요. 🌱

해당 solution 함수 상단에 필수적으로 삽입되어야하는 주석 문구가 있습니다!
00-해답-예시.js 파일을 확인하시어 추가 부탁드립니다 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

상단에 주석 추가했습니다! 👍

Comment on lines +53 to +60
Copy link
Owner

Choose a reason for hiding this comment

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

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

변수에 값을 담아 바로 return 을 하는 형식이기 때문에, 불필요한 변수 선언이나 else 문 없이 early return 패턴을 사용하시면 더 깔끔한 코드가 될 것 같다고 생각합니다!
참고링크

Suggested change
let fee = 0
if (feeInfo[0] >= totalTime) {
fee = feeInfo[1]
} else {
fee =
feeInfo[1] + Math.ceil((totalTime - feeInfo[0]) / feeInfo[2]) * feeInfo[3]
}
return fee
if (feeInfo[0] >= totalTime) return feeInfo[1]
return feeInfo[1] + Math.ceil((totalTime - feeInfo[0]) / feeInfo[2]) * feeInfo[3]
Comment on lines +23 to +28
Copy link
Owner

Choose a reason for hiding this comment

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

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

해당 문제를 풀어보지 못했지만, 만약 info를 split한 배열의 각 인덱스의 값이 각각 의미가 있는 값이라면,
아래 calculateTotalTime 에서 사용하였던 것 처럼 변수명을 명시해주신다면 더 가독성이 좋지 않을까 생각합니다!
const [info[0]을 위한 변수명, info[1]을 위한 변수명, info[2]를 위한 변수명] = val.split(' ')

사실 코테에서는 그렇게까지 할 여유가 없다고 생각합니다! 리뷰로만 보아주세요😁

@codeisneverodd
Copy link
Owner

@ljw0096 내용 확인했습니다!! 기여해주셔서 감사합니다 😁

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

Labels

None yet

2 participants