-
-
Notifications
You must be signed in to change notification settings - Fork 248
[geegong] Week 02 solutions #1751
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
Conversation
@jangwonyoon 님 안녕하세요~~ 코드 리뷰 너무 늦었지만 부탁드리겠습니다 🙇♀️ |
public List<List<Integer>> threeSum(int[] nums) { | ||
|
||
// 중복되는 값은 없어야 하기에 HashSet 으로 result | ||
HashSet<List<Integer>> result = new HashSet<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3sum의 문제의 포인트는 중복된 값을 필터링 하는게 주요한 요소라고 생각했는 데, hashMap을 사용해서 중복 값을 필터링한 부분이 굉장히 인상적이네요.
// Key : 배열 원소 , value : List<String> 인덱스들 | ||
// elementMap 은 two pointer 의 값을 더한 값에서 0이 되기 위한 요소를 찾기위해 사용될 것임 | ||
// tc : O(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상세한 주석 및 시간 복잡도 계산 좋습니다.
// leftIndex : 0에서 부터 시작하는 index | ||
// rightIndex : nums.length - 1에서부터 감소하는 index | ||
// leftIndex > rightIndex 되는 순간까지만 for문을 돌 것이다. | ||
// tc : O(N^2 / 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
시간 복잡도에서 / 2
는 큰 의미가 없어보여요. 결국에는 동일한 tc: O(N^2)인데 어떻게 생각하시나요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
시간 복잡도에서
/ 2
는 큰 의미가 없어보여요. 결국에는 동일한 tc: O(N^2)인데 어떻게 생각하시나요??
네넵, 맞습니다~ 말씀하신것 처럼 n^2 인데 루프가 반으로 나눠져서 그만 저렇게 적어버렸네요 ^^;;
꼼꼼한 확인 감사드려요~~ 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 꼼꼼한 주석 처리 덕분에 읽기 좋았습니다. 다만 전반적으로 코드 가독성과 일관된 스타일 측면에서 개선할 수 있는 여지가 보여 코멘트 남깁니다.
newOne.add(nums[leftIndex]); | ||
newOne.add(nums[rightIndex]); | ||
newOne.add(nums[thirdIndex]); | ||
result.add(newOne.stream().sorted().toList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 for문 내부에서, List를 정렬을 매번해서 연산을 진행하는 데, 이는 불필요한 연산 낭비로 보여요.
처음부터 two pointer 알고리즘을 떠올려서 정렬 + 투포인터 접근으로 한다면, 연산을 줄일수있을거 같은데 어떻게 생각하실까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오오.. 미리 nums 를 정렬해서 two pointer 로 돌린다면 마찬가지로 result 에 중복되는게 있는지 체크를 해야되지 않을까 싶은 생각도 드는데 🧐
아니면 아래처럼 중복되는걸 미리 체크하는것도 할 수 있지 않을까 싶은데..
newOne 라는 list 를 만들어서 각 원소들을 add 하는건 해야되지만 newOne.sorted() 처리는 안해도 되겠네용👍
List<Integer> newOne = new ArrayList<>();
newOne.add(nums[leftIndex]);
newOne.add(nums[rightIndex]);
newOne.add(nums[thirdIndex]);
if (!result.contains(newOne)) {
result.add(newOne);
}
for (int index=0; index<nums.length; index++) { | ||
if (index == 0) { | ||
result[index] = 1; | ||
continue; | ||
} | ||
|
||
result[index] = accumulatedProduct * nums[index - 1]; | ||
accumulatedProduct = result[index]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (int i = 0; i < nums.length; i++) {
result[i] = prefixProduct;
prefixProduct *= nums[i];
}
위와 같은 방식으로 들여쓰기를 하면, 가독성 측면에서 더욱 좋을거 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉 천재이십니다👍👍
전 너무 제 생각대로 코드를 짠거 같아 부끄럽네요ㅋㅋ
답안 제출 문제
작성자 체크 리스트
In Review
로 설정해주세요.검토자 체크 리스트
Important
본인 답안 제출 뿐만 아니라 다른 분 PR 하나 이상을 반드시 검토를 해주셔야 합니다!