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

refactor: 엑셀 메서드 분리 #1041

Merged
merged 6 commits into from
Dec 11, 2024
Merged

Conversation

dradnats1012
Copy link
Contributor

🔥 연관 이슈

  • close #이슈번호

🚀 작업 내용

  1. 엑셀 생성 메서드 분리를 했습니다

💬 리뷰 중점사항

동시성 제어 어노테이션을 적용하려고 했는데 이상하게도 적용이 되지 않는 상황을 발견해서 해당 문제는 일단 보류하겠습니다.

@dradnats1012 dradnats1012 self-assigned this Nov 18, 2024
@github-actions github-actions bot added the 리팩터링 리팩터링을 위한 이슈입니다 label Nov 18, 2024
@dradnats1012 dradnats1012 requested review from Choon0414, songsunkook, kih1015, ImTotem and DHkimgit and removed request for songsunkook November 18, 2024 06:14
Copy link

github-actions bot commented Nov 18, 2024

Unit Test Results

343 tests   342 ✔️  1m 20s ⏱️
  41 suites      1 💤
  41 files        0

Results for commit c508138.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@DHkimgit DHkimgit left a comment

Choose a reason for hiding this comment

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

잘 읽었습니다! 엑셀 생성하는 라이브러리는 잘 모르지만 generateDiningExcel 메서드 로직을 이해하기 더 쉬워진 것 같아요!

Comment on lines 137 to 142
LocalDate limitDate = LocalDate.of(2022, 11, 29);
LocalDate today = LocalDate.now();

if (startDate.isBefore(limitDate) || endDate.isBefore(limitDate)) {
throw new DiningLimitDateException("2022/11/29 식단부터 다운받을 수 있어요.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

limitDate 상수로 빼는건 어떠신가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은것 같아용 👍

Copy link
Contributor

@kih1015 kih1015 left a comment

Choose a reason for hiding this comment

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

술술 잘 읽히네요 good~

Copy link
Contributor

@Choon0414 Choon0414 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!
오타 몇 가지만 확인해주세요.

}

private void addHeaderRow(Sheet sheet, CellStyle headerStyle) {
String[] headers = {"날짜", "타입", "코너", "칼로", "메뉴", "이미지", "품절 여부", "변경 여부"};
Copy link
Contributor

Choose a reason for hiding this comment

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

오타 있어요. 칼로 -> 칼로리

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 (int i = 0; i < EXCEL_COLUMN_COUNT; i++) {
for (int i = 0; i < 8; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

상수에서 다시 숫자로 돌아온 이유가 있나요?

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 (startDate.isAfter(endDate)) {
throw new StartDateAfterEndDateException("시작일은 종료일 이전으로 설정해주세요.");
throw new StartDateAfterEndDateException("시작일은 종료일 이전으로 설정해주세요");
Copy link
Contributor

Choose a reason for hiding this comment

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

점이 다시 빠졌어요!

@dradnats1012 dradnats1012 merged commit bc82893 into develop Dec 11, 2024
4 checks passed
@dradnats1012 dradnats1012 deleted the refactor/coop-excel-refactor branch December 11, 2024 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
리팩터링 리팩터링을 위한 이슈입니다
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants