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

[GangBean] Week 1 #630

Merged
merged 6 commits into from
Dec 14, 2024
Merged

[GangBean] Week 1 #630

merged 6 commits into from
Dec 14, 2024

Conversation

GangBean
Copy link
Contributor

@GangBean GangBean commented Dec 4, 2024

답안 제출 문제

체크 리스트

  • PR을 프로젝트에 추가하고 Week를 현재 주차로 설정해주세요.
  • 바로 앞에 PR을 열어주신 분을 코드 검토자로 지정해주세요.
  • 문제를 모두 푸시면 프로젝트에서 Status를 In Review로 설정해주세요.
  • 코드 검토자 1분 이상으로부터 승인을 받으셨다면 PR을 병합해주세요.

@GangBean GangBean added the java label Dec 4, 2024
@GangBean GangBean requested a review from kdh-92 December 4, 2024 11:37
@GangBean GangBean self-assigned this Dec 4, 2024
@GangBean GangBean requested a review from a team as a code owner December 4, 2024 11:37
Comment on lines +9 to +10
s = s.toLowerCase();
s = s.replaceAll("[^a-z0-9]", "");
Copy link
Contributor

@kdh-92 kdh-92 Dec 4, 2024

Choose a reason for hiding this comment

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

  • String(불변객체)에 toLowerCase(), replaceAll()을 기존 s에 재할당하는 방식보다는 별도의 변수명에 할당하는 방식 고려
    • String(불변객체)를 잘 이해하고 쓰고있다면 문제는 없습니다.
  • Character.isLetterOrDigit() 같은게 있다고 알아두면 좋을 것 같습니다 :)

Copy link
Contributor Author

@GangBean GangBean Dec 5, 2024

Choose a reason for hiding this comment

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

@kdh-92 안녕하세요! 스터디 기간 동안 잘 부탁드립니다:) 피드백주신 부분 관련 생각을 여쭤보고 싶습니다~

  • String 타입 입력 변수에 재할당하는 것을 지양해야하는 부분은 어떤걸 고려해야 할까요?? 입력인자의 참조값을 잃어버려 재 참조가 불가능해지는 것을 고려하는 것일까요??
  • Character.isLetterOrDigit() 을 사용하는 방식은 아래와 같은 사용을 의도한 것이실까요?
while (left < right) {
  if (!Character.isLetterOrDigit(s.charAt(left))) left++;
  if (!Character.isLetterOrDigit(s.charAt(right))) right--;
  continue;
  if(s.charAt(left) != s.charAt(right)) return false;
}

좋은 피드백 감사드립니다! 😄

Copy link
Contributor

@kdh-92 kdh-92 Dec 5, 2024

Choose a reason for hiding this comment

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

@GangBean 안녕하세요 :) 저도 잘 부탁드립니다!

  • String 타입 불변 객채에 재할당하기보다는 별도의 변수명을 쓰면 좋지 않을까? 라는 개인적인 의견으로 작성했던 것 같습니다!
    • ex) 변수명을 s 보다는 lowerCaseStr 같은걸 사용해서 의도를 알 수 있게 사용하는 방식
    • 현재 상황에서는 기존 값이 변경되어도 상관없기에 위 방식은 문제가 없을 것 같습니다 :)
  • Character.isLetterOrDigit() 사용 방식은 작성해주신게 맞습니다!
    • 사실 저도 모르고 있다가 toLowerCase(), replaceAll() 말고 다른 방식은 없을까? 하며 찾다가 알게 된 내용이라 공유하고 싶었는데 위에 쓴 문장에는 문제가 있는 것 같네요..! (사용하면 좋을 것 같다 -> 이런 방식도 있지 않을까? 입니다!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • 저도 제안해주신 두번째 방식이 확실히 더 깔끔하다고 생각합니다! 좋은 내용 공유 감사드려요~
  • (사용하면 좋을 것 같다 -> 이런 방식도 있지 않을까? 입니다!) 세심하신 분이시군여 ㅎㅎ 전혀 거슬리지 않으니 맘껏 피드백주셔도 됩니당 😸 신경써주셔서 감사해요~

Comment on lines +27 to +35
Map<Integer, List<Integer>> countToNum = new HashMap<>();
for (Map.Entry<Integer, Integer> entry: numToCount.entrySet()) {
List<Integer> numList = countToNum.getOrDefault(entry.getValue(), new ArrayList<>());
numList.add(entry.getKey());
countToNum.put(entry.getValue(), numList);
} // O(logN): because sum of all counts is equal to N

List<Integer> countRank = countToNum.keySet().stream().sorted(Collections.reverseOrder()).collect(Collectors.toList());
// O(MlogM) where N = M(M+1) / 2
Copy link
Contributor

Choose a reason for hiding this comment

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

PriorityQueue 대신 위 로직을 적용한 것으로 이해되는데 맞을까요?.?
(혹시 맞다면 PriorityQueue 대신 저 로직을 쓰신 이유도 있는지 궁금합니다!)


class Solution {
public int longestConsecutive(int[] nums) {
Set<Integer> set = Arrays.stream(nums).boxed().collect(Collectors.toSet());
Copy link
Contributor

Choose a reason for hiding this comment

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

contains-duplicate 쪽에서도 Stream을 사용하셨었는데 Stream에 익숙하신가요?
(저는 아직 문제보고 바로 적용하는게 어렵더라구요 ㅠㅠ)

Copy link
Contributor Author

@GangBean GangBean Dec 11, 2024

Choose a reason for hiding this comment

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

stream 의도적으로 써보려고 연습중입니다 ㅎㅎ 새로 맡은 업무 코드가 대부분 stream 으로 작성되어 있어서요~
이번 스터디 기간동안 익숙해지는 걸 목표로 같이 해봐요!

Comment on lines +39 to +45
int rank = 0;
while (idx < k) { // O(k) <= O(N)
for (int num : countToNum.get(countRank.get(rank++))) {
ret[idx++] = num;
}
}
return ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

저는 요 부분을 for문으로 했을 것 같은데 while문을 사용하셨네요! 👍

chatgpt에 물어보다보니 Bucket 방식 (Array + List) 이 있던데 빈도를 인덱스로 두고 해당하는 값들을 배열로 저장 처리하는 방식이 있더라구요. (장점 : 시간 복잡도를 O(N)으로 줄일 수 있습니다!)

추가적으로 while + for or for + for를 사용해 ret (result)를 추출하는 부분을 1개의 for문으로 해결할 수 있더라구요!

코드 간단하게 공유드립니당 :)

// 빈도를 인덱스로 사용해 저장하는 방식
List<Integer>[] buckets = new List[nums.length + 1];
for (int key : numToCount.keySet()) {
    int freq = numToCount.get(key);
    if (buckets[freq] == null) {
        buckets[freq] = new ArrayList<>();
    }
    buckets[freq].add(key);
}

// 1개의 for 문으로 결과물 처리
List<Integer> result = new ArrayList<>();
for (int i = buckets.length - 1; i >= 0 && result.size() < k; i--) {
    if (buckets[i] != null) {
        result.addAll(buckets[i]);
    }
}
return result.stream().mapToInt(Integer::intValue).toArray();

@GangBean GangBean requested a review from kdh-92 December 13, 2024 11:53
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.

@GangBean 님과 @kdh-92 님께서 피드백을 주고 받으시는 모습이 너무 보기 좋은 것 같습니다. PR을 병합하기에 충분히 좋은 코드를 제출해주셨다고 생각되어 스터디 일정 관리 차원에서 우선 승인드리겠습니다. 두 분은 계속 편하게 나머지 대화를 나누셔도 무방할 것 같아요 :)

@GangBean GangBean merged commit ed353c4 into DaleStudy:main Dec 14, 2024
1 check 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.

3 participants