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

[HodaeSsi] week 06 #924

Merged
merged 5 commits into from
Jan 19, 2025
Merged

[HodaeSsi] week 06 #924

merged 5 commits into from
Jan 19, 2025

Conversation

HodaeSsi
Copy link
Contributor

@HodaeSsi HodaeSsi commented Jan 19, 2025

답안 제출 문제

체크 리스트

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

@HodaeSsi HodaeSsi requested review from neverlish and EgonD3V January 19, 2025 07:42
@HodaeSsi HodaeSsi requested a review from a team as a code owner January 19, 2025 07:42
@github-actions github-actions bot added the py label Jan 19, 2025
Copy link
Contributor

@EgonD3V EgonD3V left a comment

Choose a reason for hiding this comment

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

전체적으로 알고리즘의 로직이 깔끔해서 이해하기 쉬웠습니다. 이번 주도 수고많으셨습니다. 다음 주도 화이팅!

# 공간 복잡도 : O(1)
# 문제 유형 : Two Pointer
class Solution:
def maxArea(self, height: List[int]) -> int:
Copy link
Contributor

Choose a reason for hiding this comment

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

가능하면 4 space indent로 맞춰주시면 좋을 것 같습니다. 파이썬의 PEP8에서 권고하는 사항입니다. https://peps.python.org/pep-0008/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오오 자세한 리뷰 감사합니다!

if char not in node:
node[char] = {}
node = node[char]
node['$'] = True
Copy link
Contributor

Choose a reason for hiding this comment

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

저 라면 '$' 처럼 클래스의 다른 메서드에서도 사용하는 상수는 클래스 레벨에서 선언해서 공유할 것 같습니다

Comment on lines +20 to +29
def dfs(self, i, node, word):
if i == len(word):
return '$' in node
if word[i] == '.':
for child in node.values():
if isinstance(child, dict) and self.dfs(i + 1, child, word):
return True
if word[i] in node:
return self.dfs(i + 1, node[word[i]], word)
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.

dfs 메서드는 search 메서드에서만 사용하므로 search 메서드 내부 레벨에서 선언하면 좋을 것 같습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그런 코드들을 보긴했는데,
제가 자바쟁이라 함수안에 함수 정의가 어색하긴 하네요😅


for char in s:
if char in (')', '}', ']'):
if not stack or stack.pop() != {')': '(', '}': '{', ']': '['}[char]:
Copy link
Contributor

Choose a reason for hiding this comment

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

이 라인은 가독성도 그렇고 임시 변수가 for문을 조회하며 계속 생성될 텐데, 저라면 외부 scope에서 변수로 선언해두고 사용할 것 같습니다

return '$' in node
if word[i] == '.':
for child in node.values():
if isinstance(child, dict) and self.dfs(i + 1, child, word):
Copy link
Contributor

Choose a reason for hiding this comment

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

isinstance를 사용하기 보다, boolean이나 None을 활용하는 방식으로 Trie를 개선해 볼 수 있을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

처음엔 그렇게 했다가
dict의 자식 원소로 dict 타입과 boolean 타입이 혼재되어서
반복, iterate에서 에러가 발생하더라구요?🤔

@HodaeSsi HodaeSsi merged commit 89edeec into DaleStudy:main Jan 19, 2025
3 checks 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