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

[pepper] WEEK 2 Solution #363

Merged
merged 6 commits into from
Aug 24, 2024
Merged

[pepper] WEEK 2 Solution #363

merged 6 commits into from
Aug 24, 2024

Conversation

Copy link
Contributor

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

@DaleSeo DaleSeo requested a review from gitsunmin August 23, 2024 23:56
Comment on lines 12 to 13
count[sChar] = (count[sChar] || 0) + 1;
count[tChar] = (count[tChar] || 0) - 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: 이 문제의 경우 무방하지만, 일반적으로는 || 대신에 ??를 사용하는 것이 더 안전하지 않을까 생각이 들었습니다.

Suggested change
count[sChar] = (count[sChar] || 0) + 1;
count[tChar] = (count[tChar] || 0) - 1;
count[sChar] = (count[sChar] ?? 0) + 1;
count[tChar] = (count[tChar] ?? 0) - 1;

Copy link
Contributor Author

@whewchews whewchews Aug 24, 2024

Choose a reason for hiding this comment

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

감사합니다! 말씀해주신대로 ?? 을 사용하는게 좋을 것 같습니다.
이런 코드에서는 원하는 값이 없는 경우에, 기본값을 사용할 목적이니 JS의 falsy한 값을 모두 처리하는 ||(논리OR연산자)가 아닌 null이나 undefined 값만 처리하는 ??(널병합연산자) 를 사용하는게 더 목적에 맞고 안전하게 처리해줄 수 있겠네요!

@whewchews whewchews merged commit 5a776b6 into DaleStudy:main Aug 24, 2024
1 of 2 checks passed
@DaleSeo
Copy link
Contributor

DaleSeo commented Aug 24, 2024

@whewchews 님, Week 설정을 안 해주셔서 이번 주 풀이로 안 잡히셨더라고요. 다음 PR 부터 Week 설정 좀 꼭 부탁드리겠습니다.

Shot 2024-08-24 at 06 48 18@2x

@whewchews
Copy link
Contributor Author

whewchews commented Aug 24, 2024

네, Week 체크하겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants