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

[HaJunYoo] Week 2 #376

Merged
merged 8 commits into from
Aug 28, 2024
Merged

[HaJunYoo] Week 2 #376

merged 8 commits into from
Aug 28, 2024

Conversation

@HaJunYoo HaJunYoo marked this pull request as ready for review August 25, 2024 10:29
@obzva
Copy link
Contributor

obzva commented Aug 26, 2024

오 안녕하세요 하준님 혹시 리뷰 요청을 제게 주신 이유가 있을까요?
일단은 제가 리뷰하겠습니다

@HaJunYoo
Copy link
Contributor Author

저번에 리뷰 받았을 때, 꼼꼼하게 리뷰 주신 게 기억나서 부탁 드렸습니다 ㅎㅎ
같은 언어로 푸신 분의 리뷰를 부탁드리는게 나았을까요?

@obzva
Copy link
Contributor

obzva commented Aug 26, 2024

아하 그러셨군요 ㅎㅎㅎ 감사합니다 :D
같은 언어로 푸신 분의 리뷰를 부탁드리는게 나았을까요? <- 아니요~ 제가 알기로는 리뷰어 지정은 바로 아래 PR 작성자분께 부탁드리는 것으로 알고 있어서요!
week3 부터는 그렇게 하시는 것도 좋을 것 같아요
이번꺼는 제가 하겠습니다!

Copy link
Contributor

@obzva obzva left a comment

Choose a reason for hiding this comment

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

하준님 풀이 잘 보았습니다 :D

Solution이 여러개 있는 풀이는 답안을 여러개 제출하신 것인지, 아니면 Solution1 -> Solution N으로 갈 수록 최적화하신 건지 잘 모르겠어서 일단 모든 풀이에 대해 리뷰를 진행했습니다

Big-O 분석이 익숙하지 않으신 것 같다는 인상을 받았는데, 시간복잡도와 공간복잡도를 어떻게 계산하셨는지 좀 더 상세히 적어주시면 저도 도움드리기 좋고 하준님의 분석 능력 향상에도 좋을 것 같습니다

if not preorder or not inorder:
return None

val = preorder.pop(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

pop(0)시간복잡도는 O(N)입니다.
해당 method의 시간복잡도 때문에 알고리즘 전체 시간 복잡도도 증가할 것으로 추측되는데, 다른 방안을 고려해보는 것도 좋을 것 같아요

# space complexity: O(1)
def countBits(self, n: int) -> List[int]:
list = [i for i in range(n + 1)]
result = [bin(num).count('1') for num in list]
Copy link
Contributor

Choose a reason for hiding this comment

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

전체 시간 복잡도에 str.count() method의 시간복잡도 또한 고려해보는게 좋을 것 같습니다

# time complexity: O(n)
# space complexity: O(1)
def encode(self, strs):
return ":;".join(strs)
Copy link
Contributor

Choose a reason for hiding this comment

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

image ':', ';' 둘 모두 ASCII 256에 포함되어 있는 문자라서 아래 테스트 케이스에 대해서는 오류가 발생할 것 같아요
Encode
input: ["asdf:;:;:;", "asdfasdf"]
output: "asdf:;:;:;:;asdfasdf"

Decode
input: "asdf:;:;:;:;asdfasdf"
output: ["asdf", "", "", "", "asdfasdf"]

Comment on lines +8 to +24
char_map = defaultdict(int)
for s1 in s:
char_map[s1] += 1

contrast_map = defaultdict(int)
for t1 in t:
contrast_map[t1] += 1

for key, val in char_map.items():
contrast_val = contrast_map[key]
if contrast_val != val:
return False

for key, val in contrast_map.items():
char_val = char_map[key]
if char_val != val:
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

char_map, contrast_map <- 이렇게 hashmap을 두 개 선언하여 st의 알파벳 개수를 세는 것도 괜찮지만
아래처럼 char_map 하나만 사용하면 사용하는 공간을 반으로 줄이고, 결과를 확인하는 반복문도 반으로 줄일 수 있을 것 같아요

char_map = defaultdict(int)

for s1 in s:
    char_map[s1] += 1
for t1 in t:
    char_map[t1] -= 1

for key in char_map:
    if char_map[key]:
        return False
return True


class Solution:
# Time complexity: O(n)
# Space complexity: O(n)
Copy link
Contributor

Choose a reason for hiding this comment

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

alphabet의 개수는 26개로 고정되어 있기 때문에 hashmap의 key도 26을 넘을 수 없다는 걸 알수 있습니다
따라서 공간 복잡도는 사실상 O(1)이라고 봐도 무방할 것 같아요

Copy link
Contributor

@wogha95 wogha95 left a comment

Choose a reason for hiding this comment

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

python 코드를 너무 오랜만에 보았지만 이해하기 쉽게 작성해주셔서 덕분에 수월하게 이해할 수 있었어요 :)
그리고 obzva 님 리뷰에 공감되는 것이 많아서 궁금한 점 위주로 남겨두었습니다!
week02 동안 모든 문제 푸시느라 고생많으셨습니다~!!


class Solution3:
# time complexity: O(n)
# space complexity: O(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Q
공간 복잡도는 답안 배열을 제외하고 O(1)로 계산하신걸까요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 그렇네요!
O(n)이 맞습니다
감사드려요 ㅎㅎ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

O(n)이 맞습니다 ㅎㅎ 제가 고려를 못했습니다

Copy link
Contributor

Choose a reason for hiding this comment

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

분석하는 사람에 따라 답안으로 제출하는 Object의 공간 복잡도는 고려하지 않기도 합니다.
저는 풀이에 필요한 추가적인 공간 복잡도만 계산하는 편이에요.

Comment on lines +27 to +34
def countBits(self, n: int) -> List[int]:
res = [0] * (n + 1)
msb = 1
for i in range(1, n + 1):
if i == msb * 2:
msb *= 2
res[i] = res[i - msb] + 1
return res
Copy link
Contributor

@wogha95 wogha95 Aug 26, 2024

Choose a reason for hiding this comment

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

오 이렇게도 index를 계산해서 풀 수 있겠군요! 👍

그런데 혹시 msb 네이밍 사용이 궁금합니다!
현 로직과 msb와 다른 풀이 방향이 아닐까 싶어 의문이 들었습니다🤔
제가 부족하게 이해하였다면 추가 설명을 듣고 싶습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

원래는 mhb(most highest bit)로 했다가 최상위 비트라는 네이밍으로 검색해서 수정했습니다 ㅎㅎ

@HaJunYoo HaJunYoo merged commit f0ba3e8 into DaleStudy:main Aug 28, 2024
1 check passed
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.

3 participants