Skip to content

[forest000014] Week 05 #851

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

Merged
merged 5 commits into from
Jan 10, 2025
Merged

[forest000014] Week 05 #851

merged 5 commits into from
Jan 10, 2025

Conversation

forest000014
Copy link
Contributor

@forest000014 forest000014 commented Jan 5, 2025

답안 제출 문제

체크 리스트

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

@forest000014 forest000014 requested a review from mintheon January 5, 2025 21:30
@forest000014 forest000014 self-assigned this Jan 5, 2025
@forest000014 forest000014 requested a review from a team as a code owner January 5, 2025 21:30
@github-actions github-actions bot added the java label Jan 5, 2025
Copy link
Member

@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.

5문제 모두 푸시느라 수고 많으셨습니다! 승인하는데 영향이 없는 사소한 피드백 2개 남겼습니다.

Comment on lines +17 to +25
if (prices[i] > max) {
max = prices[i];
if (max - min > ans) {
ans = max - min;
}
}
if (prices[i] < min) {
min = max = prices[i];
}
Copy link
Member

Choose a reason for hiding this comment

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

max라는 변수가 없었으면 알고리즘이 더 간단할 수 있었겠다는 생각이 들었습니다.

Suggested change
if (prices[i] > max) {
max = prices[i];
if (max - min > ans) {
ans = max - min;
}
}
if (prices[i] < min) {
min = max = prices[i];
}
if (prices[i] - min > ans) {
ans = prices[i] - min;
}
if (prices[i] < min) {
min = prices[i];
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

다시 보니 굳이 불필요하게 max라는 변수를 쓸 필요가 없었네요... sweat_smile:
한번 사고의 흐름이 한 방향으로 생기고 나니, 코드를 간결하게 하려고 다시 봐도 잘 안 보이는 것 같네요 ㅠㅠ

Comment on lines +29 to +37
List<List<String>> ans = new ArrayList<>();
for (String key : map.keySet()) {
ans.add(new ArrayList<>());
for (String str : map.get(key)) {
ans.get(ans.size() - 1).add(str);
}
}

return ans;
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 사소한 자료 구조 변환은 한 줄로 처리해주시면 위에 더 중요한 코드를 읽는데 더 집중할 수 있을 것 같아요.

Suggested change
List<List<String>> ans = new ArrayList<>();
for (String key : map.keySet()) {
ans.add(new ArrayList<>());
for (String str : map.get(key)) {
ans.get(ans.size() - 1).add(str);
}
}
return ans;
return new ArrayList<>(map.values());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 이 부분은 map을 단순히 List로 바꾼 것은 아니고, 1-depth로 되어있는 map을 2-depth로 변환하기 위한 코드입니다
다시 보니 달레님께서 말씀해주신 게 그 기능이네요! 이런 식으로 할 수 있다는 걸 새롭게 배웠습니다 😄

@forest000014 forest000014 merged commit 777fd88 into DaleStudy:main Jan 10, 2025
3 checks passed
@forest000014 forest000014 changed the title [forest000014] Week 5 [forest000014] Week 05 Feb 6, 2025
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.

2 participants