Skip to content
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

[YeomChaeeun] Week 1 #673

Merged
merged 10 commits into from
Dec 14, 2024
Merged

[YeomChaeeun] Week 1 #673

merged 10 commits into from
Dec 14, 2024

Conversation

YeomChaeeun
Copy link
Contributor

@YeomChaeeun YeomChaeeun commented Dec 10, 2024

답안 제출 문제

체크 리스트

  • 우측 메뉴에서 PR을 Projects에 추가해주세요.
  • Projects의 오른쪽 버튼(▼)을 눌러 확장한 뒤, Week를 현재 주차로 설정해주세요.
  • 바로 앞에 PR을 열어주신 분을 코드 검토자로 지정해주세요.
  • 문제를 모두 푸시면 프로젝트에서 StatusIn Review로 설정해주세요.
  • 코드 검토자 1분 이상으로부터 승인을 받으셨다면 PR을 병합해주세요.

@YeomChaeeun YeomChaeeun requested a review from a team as a code owner December 10, 2024 14:15
@github-actions github-actions bot added the ts label Dec 10, 2024
@HC-kang
Copy link
Contributor

HC-kang commented Dec 10, 2024

안녕하세요 @YeomChaeeun 님!
반복적으로 파일명 검사가 실패하신다면 아래 링크를 확인해주세요!
#664 (comment)

@YeomChaeeun YeomChaeeun changed the title [chae] Week 1 [YeomChaeeun] Week 1 Dec 10, 2024
@YeomChaeeun YeomChaeeun requested a review from eunhwa99 December 13, 2024 14:05
Copy link
Contributor

@HC-kang HC-kang left a comment

Choose a reason for hiding this comment

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

안녕하세요, @YeomChaeeun 님! 첫주차 고생 많으셨습니다!
시도하신 방법을 하나씩 정리해주신 부분에서 사고 흐름을 볼 수 있어서 정말 좋았습니다!

가능하시다면 다음 주차부터는 각각의 복잡도 분석도 남겨주시면 더 좋지 않을까요?!
다음 주에도 좋은 풀이 기대하겠습니다!!

let obj={}

for(let i = 0; i < nums.length; i++) {
console.log('nums >>', nums[i], 'obj >>', obj)
Copy link
Contributor

@HC-kang HC-kang Dec 14, 2024

Choose a reason for hiding this comment

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

제출시에는 지워주시는 편이 좋을 것 같아요~🧹
아마 다른 부분은 해주신걸로 보아 이부분만 놓치신것 같네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 수정하겠습니다! 감사합니다!

// =========

let longest = 0;
let temp = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

중요하진 않은 내용이지만, 변수명에 의미를 담아주시면 가독성에 도움이 될 것 같아요.
여기서는 현재까지 연속된 수를 의미하니까, currentSequence 혹은 longest 와 맞추어 current 등은 어떨까요?

}
}

// i가 마지막인 경우 for문의 else문을 타지 않으므로 다시 한번 체크함
Copy link
Contributor

Choose a reason for hiding this comment

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

for (let i = 1; i < nums.length; i++) {
        if (nums[i] === nums[i - 1]) {
            continue;
...

위와 같은 형태로 +1이 아니라 -1로 접근한다면 아래의 추가적인 확인 로직이 필요없지 않을까요?

l++;
}

return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

불필요한 순회를 막기 위해 l을 사용하신것 같아, 최적화도 고려하신것 같아 좋네요!
이 부분은 취향의 문제이지만 아래와 같은 함수형 풀이는 어떻게 생각하시나요?

return Object.entries(frequencyMap)
        .sort((a, b) => b[1] - a[1])
        .slice(0, k)
        .map(entry => Number(entry[0]));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

함수형 풀이로 소스를 더 간략하게 작성할 수 있겠네요!
좋은 의견 감사합니다! ☺️


word = word.replace(reg, '');
for(let i = 0; i < word.length; i++) {
for(let j = word.length-i-1; j >= word.length-i-1; j--) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 루프는 사실상 한번밖에 돌지 않는것같은데, 혹시 이렇게 작성하신 이유가 있을까요?

*/
function isPalindrome(s: string): boolean {
let word = s.toLowerCase();
const reg = /[\{\}\[\]\/?.,;:|\)*~`!^\-_+<>@\#$%&\\\=\(\'\" ]/g;
Copy link
Contributor

Choose a reason for hiding this comment

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

음.. 모든 아스키 코드를 고려하신 것 같은데요,
문제의 본문에서 ...removing all non-alphanumeric characters...라는 부분이 있습니다.
따라서 위 조건과 6 line을 통해, 아래와 같이 간소화 하실 수 있지 않을까요?

const reg = /[^a-z0-9]/g;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

모든 특수문자를 지워야한다고 생각했던거 같습니다 😅
꼼꼼히 봐주셔서 감사합니다. 간소화 해보겠습니다 ^^

@YeomChaeeun YeomChaeeun merged commit f4eb476 into DaleStudy:main Dec 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants