Repository navigation
Conversation
|
🤖 AI 리뷰(알고리즘 패턴, 시간/공간 복잡도, 학습 현황) 결과는 Checks 탭에서 볼 수 있어요. |
novacat25
changed the title
There was a problem hiding this comment.
안녕하세요 @novacat25 님 :)
지난 온보딩 때도 리뷰를 맡았었는데, 이번에도 맡게 되어 반갑습니다. TS와 점점 가까워지는 중입니다...ㅎㅎ
저도 Python에서 처음엔 DFS(O(n²))로 풀었다가 dict로 O(n)까지 개선했는데, 비슷한 고민의 흔적이 담겨 있어서 반가웠어요.
Map을 Python dict와 비교해 보았습니다.
- map.set(key, value) ↔ d[key] = value
- map.get(key) ↔ d.get(key)
- map.has(key) ↔ key in d
has()로 key가 있는지 확인했더라도 TS는 get()의 반환값을 여전히 number | undefined로 판단하는데, 이를 !로 확정한 이유를 옆에 주석으로 설명해 주셔서 이해하기 쉬웠습니다.
궁금한 점이 하나 있습니다
Q) if 안에서 break로 빠져나가니 별도의 else 없이 그 아래에 map.set(currentNum, i)를 두어도 될 것 같은데 else를 쓰신 이유가 있을까요?
AI 시대일수록 언어를 학습하고 코드로 소통하는 힘이 중요하다고 느끼는데, 서로 다른 언어로 같은 접근을 찾아 나가는 과정이 굉장히 의미 있어서 스터디에 참여하길 정말 잘했다는 생각이 드네요.
이번 주 과제를 정말 빠르게 수행하셨는데, 수고 많으셨습니다 💪
There was a problem hiding this comment.
안녕하세요, 리뷰 감사합니다!
저도 오히려 Python을 다룰 기회가 없어서 Python 메소드로 보니까,
저야말로 관점의 자극이 되어서 기쁩니다!
곰곰이 생각해보니 지적대로네요.
break나 return으로 강제로 빠져나가는 구문이 있으니, else 없어도 되긴 했네요..
else를 쓴 이유는 돌이켜보니 별 건 없고,
약간 조금 더 확실한 안전빵 조건으로 두고 싶지 않았나 하는 마음으로 썼습니다!
실무에서는 확실히 if문만 써서 가독성을 신경쓰긴 했지만,
코딩 테스트만 보면 조금 보수적으로 써서 그럤던 것 같네요.
물론, 조금 더 가독성을 챙긴다면 저 역시 지우는 것에는 찬성합니다..
아무튼, 이번 주도 물론이고 쭉 파이팅 해봐요!
There was a problem hiding this comment.
빠른 피드백 감사합니다🙂
저도 다른 문제 파이썬으로 풀어보고
나머지 문제들도 리뷰 남기겠습니다!🫡
201411108
left a comment
201411108
left a comment
There was a problem hiding this comment.
안녕하세요.
담당 리뷰어는 아니지만 typescript로 풀이를 하신 반가움에 리뷰를 달아봤습니다.
알고리즘과 관련된 부분은 아니고 질문과 가독성과 관련된 내용을 담았어요.
시간 괜찮으실 때 확인해봐주시면 좋을 것 같습니다.
화이팅입니다!
| /* | ||
| 시간 복잡도(Time Complexity): 평균 O(n) | ||
| 공간 복잡도(Space Complexity): 평균 O(n) | ||
| 풀이 알고리즘: JS의 HashMap / Map 이용 |
There was a problem hiding this comment.
❓궁금한 점이 있습니다! 풀이 설명에는 HashMap / Map을 사용하셨다고 적혀 있는데, 실제 코드는 Set을 활용하신 것 같아 질문드립니다.
JS/TS에서 Set과 Map 모두 내부적으로 해시 테이블을 기반으로 동작한다는 점은 같지만, 저장 방식이나 개념은 다른 자료구조로 알고 있어서요 혹시 제가 놓치고 있는 부분이 있거나 다른 의도가 있으셨던 건지 궁금합니다!
There was a problem hiding this comment.
좋은 피드백 감사합니다!
코드를 다시 확인해 보니 말씀해 주신 부분이 맞았습니다.
당시 주석을 작성할 때, Hash Table이라는 내부 탐색 구조와 HashMap / Map이라는 자료 구조의 개념을 혼용했던 것 같습니다.
실제 구현은 Key-Value 쌍을 저장하는 Map이 아니라,
지적하신 대로 중복되지 않는 값들을 저장하고 존재 여부를 확인하는 Set을 사용한 풀이가 맞습니다.
JavaScript의 Map과 Set 모두 해시 테이블과 같은 효율적인 탐색 구조를 활용할 수 있다는 공통점이 있기는 하지만,
엄밀하게는 서로 다른 컬렉션이기 때문에 주석을 JavaScript Set으로 수정하는 것이 정확하겠습니다.
덕분에 자료구조의 개념을 다시 정리할 수 있었습니다.
더 이상 Hash Table, Hash Map, Hash Set 이름으로 헷갈리지 않도록 확실히 짚고 넘어가야 겠네요.
There was a problem hiding this comment.
@201411108 님! 담당 리뷰어가 아니심에도 불구하고 적극적으로 피드백해주셔서 감사합니다:)
정말 상세히 리뷰해주셔서 제가 오히려 도움이 많이 되었습니다
코멘트들이 많이 달려서 저도 참고하여 리뷰 작성하겠습니다.
감사합니다
There was a problem hiding this comment.
달레님께서도 리트코드 채팅에 올려주셨지만 블로그에 Hash, hash map에 대해 정리하신 부분 잘 읽었습니다
덕분에 저도 개념 정리하고 갑니다:)감사합니다
| for (let i = 0; i < freqArr.length; i++) { | ||
| const [num, freq] = freqArr[i] | ||
| buckets[freq].push(num) | ||
| } |
There was a problem hiding this comment.
💬 for of를 쓰는 방식으로 가독성을 개선해보는 것도 좋을 것 같습니다!
| for (let i = 0; i < freqArr.length; i++) { | |
| const [num, freq] = freqArr[i] | |
| buckets[freq].push(num) | |
| } | |
| for (const [num ,freq] of freqArr) { | |
| buckets[freq].push(num) | |
| } |
There was a problem hiding this comment.
오, 확실히 이 부분도 지적대로십니다.
index를 통한 직접적인 탐색의 이유나 실익이 없으므로, 제시해주신 코드가 훨씬 더 깔끔하네요.
현직 FE 엔지니어 분이셨군요... |
parkhojeong
left a comment
parkhojeong
left a comment
There was a problem hiding this comment.
수고하셨습니다! two-sum 풀이에 대해 리뷰 했습니다~
| let result = -1 | ||
|
|
||
| for (let i = 1; i < n; i++) { | ||
| result = Math.max(nums[i] + secondLast, last) |
There was a problem hiding this comment.
저는 dp[i]를 i번째 집을 반드시 방문해서 훔치는 식으로 정의해서 nums[i] + max(dp[i-2], dp[i-3])로 풀었는데,
i번째 집까지의 최대값으로 정의하셨네요..
저도 dp로 풀어서 풀이가 굉장히 비슷하다고 생각했는데 정의에서 차이가 나네요
이렇게 정의하니 i-3을 볼 필요가 없어진 듯하네요
There was a problem hiding this comment.
그렇군요..!
역시 사소한 디테일이 코드 짜는 사람마다 미세하게 달라지는 것 같긴 합니다.
당장 contains-duplicate 문제만 해도 Set과 length를 비교해서 원큐에 푸시는 분도 계셨던가 하면,
저처럼 명시적으로 로직을 쓰시는 분도 계셨던 것 같네요.
| const rob = (nums: number[]): number => { | ||
| const n = nums.length | ||
|
|
||
| if (n === 0) return 0 |
There was a problem hiding this comment.
문제 조건에서 nums의 길이가 1 이상이라 이 줄은 제거해도 될 듯 하네요!
There was a problem hiding this comment.
오, 습관적으로 n이 0일 때를 차단하는 식으로 코드를 짰었는데 조건 상에 1 이상이다 라고 한다면 있을 이유가 없겠네요.
감사합니다!
| const answerArr: number[] = [] | ||
| const entryArr: number[][] = [] | ||
|
|
||
| const freqMap = new Map<number, number>() |
There was a problem hiding this comment.
파이썬의 dic에 대응되는 TS의 Map코드군요!
There was a problem hiding this comment.
네, Python에서는 dict/set, Java에서는 또 구현 자료구조 별로 HashMap/HashSet/TreeMap 등으로 세분화되어 있는 것 같고...
비슷한 개념이지만 언어별로 이런 차이가 있는 게 재미있는 것 같습니다.
|
|
||
| for (const entry of freqMap.entries()) { | ||
| entryArr.push(entry) | ||
| } |
There was a problem hiding this comment.
32-34줄의 경우
entryArr로 다시 옮기지 않고 freqMap(Map)에서 for 문으로 [num, freq]를 바로 꺼낼 수 있는듯합니다!
파이썬의 dict처럼 key,value를 뽑아낼 수 있을 것 같아서 entryArr가 없어도 될 듯 합니다.
혹시 entryArr를 추가하신 이유를 여쭤봐도 될까요?
There was a problem hiding this comment.
오, 이 부분 말씀대로 바로 써지네요.
큰 이유는 따로 없었고,
iteration을 통한 순차적인 접근/처리에는 그래도 배열이 좀 더 적합한 것 같아서 배열로 변환해서 처리했었거든요.
[LOG]: "freqMap", Map (3) {1 => 4, 2 => 4, 3 => 2}
[LOG]: "entryArr", [[1, 4], [2, 4], [3, 2]]
그래서 메모리를 조금 더 써서 변환 처리 + 순차 탐색을 위한 Matrix Array 정의를 했는데...
아예 Map을 기반으로 for of iteration이 돌아지니 정의할 이유가 없어졌습니다.
날카로운 지적 감사합니다!
There was a problem hiding this comment.
이렇게 비교해주시니 더 명확하네요
JS에서 Map이 넣은 순서대로 iterate한다는 개념 다시 짚고 넘어가네요🙂
감사합니다:)
| const numberSet = new Set<number>() | ||
| let result = 0 | ||
|
|
||
| for (const val of nums) { | ||
| numberSet.add(val) | ||
| } |
There was a problem hiding this comment.
for문을 const numberSet = new Set(nums) 이렇게 간단하게 줄일 수 있을 것 같아요!
There was a problem hiding this comment.
네, 확실히 더 간소화 시킬 수 있었네요!
이 부분도 반영합니다.
| numberSet.add(val) | ||
| } | ||
|
|
||
| const setArr = [...numberSet] |
There was a problem hiding this comment.
set도 순회가 가능해서 numberSet을 바로 loop 돌려도 될 듯합니다:)
| for (const val of setArr) { | ||
| if (!numberSet.has(val - 1)) { | ||
| let cur = val | ||
| let cnt = 0 | ||
| while (numberSet.has(cur)) { | ||
| cur++ | ||
| cnt++ | ||
| } |
There was a problem hiding this comment.
저는 정렬 후 dp[i]를 nums[i]로 끝나는 연속 수열 길이로 정의한 끝점 방식으로 풀었는데,
여기서는 !numberSet.has(val - 1)로 시작점을 먼저 찾고 while로 앞으로 counting해서 정렬이 필요 없는 풀이네요
시작점에서만 카운팅해서 같은 구간을 중복해서 세지 않는 구조가 인상적이었습니다.
풀이 잘 보았습니다🙂
| 풀이 알고리즘: JavaScript Set 활용 | ||
| 풀이 History | ||
| 1트: Array.find()를 이용해 중복 여부를 탐색했으나, |
There was a problem hiding this comment.
저와 history가 굉장히 비슷해서 코멘트 남기고 갑니다 ㅠㅠ
Python의 count()로 풀었다가 TLE가 나서 set으로 바꿨습니다..
find나 count는 시간 복잡도가 크게 늘어난다는 걸 다시 한번 느끼고 가네요
There was a problem hiding this comment.
확실히, LeetCode 같은 경우에는 TLE를 되게 깐깐하게 잘 내는 것 같더라고요.
보통 코테는 로직만 통과시키면 오케이 사인을 자주 받던데...
여긴 그만큼 자료구조를 특히나 잘 생각하고 풀어야 하는만큼 기본기 다지는 부분에는 이만한 게 없는 것 같습니다.
There was a problem hiding this comment.
코테를 풀 때 플랫폼마다 기준이 상이하다보니, 통과하면 잘 푼 거겠지~ 하고 느슨하게 넘어가는 경우가 많은데,
리트코드는 기준이 엄격해서 TLE가 빡빡하다 보니 풀이를 계속 점검하게 된다는 점에서 좋은 것 같네요!
@novacat25님 ! week1주차 설정 누락된 것 같습니다
There was a problem hiding this comment.
아, 이 부분 접혀 있어서 발견이 안됐습니다.
PR 코멘트에도 GitHub Bot이 계속 서서 알려줬는데, 못 보고 있었네요 ㅎㅎ
| for (let i = 0; i < nums.length; i++) { | ||
| const currentNum = nums[i] | ||
| if (numSet.has(currentNum)) { | ||
| return true |
There was a problem hiding this comment.
인덱스 i 대신 for (const num of nums)로 바로 처리가 가능해 보입니다~!
TS를 공부하면서 알게 됐는데, 파이썬은 for ~ in nums 형태인데 TS에서는 for ~ of nums네요.
차이도 알고 갑니다👍
There was a problem hiding this comment.
오, Python에서도 비슷한 문법이 있었군요!
로직 간소화의 경우에는...역시 지적대로십니다.
index를 통한 조작의 의미가 없으므로 조금 더 간편하게 줄여보았습니다.
답안 제출 문제
작성자 체크 리스트
In Review로 설정해주세요.검토자 체크 리스트
Important
본인 답안 제출 뿐만 아니라 다른 분 PR 하나 이상을 반드시 검토를 해주셔야 합니다!