-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor: 엑셀 메서드 분리 #1041
Conversation
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.
잘 읽었습니다! 엑셀 생성하는 라이브러리는 잘 모르지만 generateDiningExcel 메서드 로직을 이해하기 더 쉬워진 것 같아요!
LocalDate limitDate = LocalDate.of(2022, 11, 29); | ||
LocalDate today = LocalDate.now(); | ||
|
||
if (startDate.isBefore(limitDate) || endDate.isBefore(limitDate)) { | ||
throw new DiningLimitDateException("2022/11/29 식단부터 다운받을 수 있어요."); | ||
} |
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.
limitDate 상수로 빼는건 어떠신가요?
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.
좋은것 같아용 👍
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.
술술 잘 읽히네요 good~
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.
고생하셨습니다~!
오타 몇 가지만 확인해주세요.
} | ||
|
||
private void addHeaderRow(Sheet sheet, CellStyle headerStyle) { | ||
String[] headers = {"날짜", "타입", "코너", "칼로", "메뉴", "이미지", "품절 여부", "변경 여부"}; |
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.
오타 있어요. 칼로
-> 칼로리
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 < EXCEL_COLUMN_COUNT; i++) { | ||
for (int i = 0; i < 8; 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.
상수에서 다시 숫자로 돌아온 이유가 있나요?
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.
오잉 상수가 없어졋네요
if (startDate.isAfter(endDate)) { | ||
throw new StartDateAfterEndDateException("시작일은 종료일 이전으로 설정해주세요."); | ||
throw new StartDateAfterEndDateException("시작일은 종료일 이전으로 설정해주세요"); |
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.
점이 다시 빠졌어요!
🔥 연관 이슈
🚀 작업 내용
💬 리뷰 중점사항
동시성 제어 어노테이션을 적용하려고 했는데 이상하게도 적용이 되지 않는 상황을 발견해서 해당 문제는 일단 보류하겠습니다.