-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: 충전기 상태 및 혼잡도를 저장하는 기능을 구현한다 #113
Conversation
#69 |
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.
수고하셨습니다
public static ChargerState from(String input) { | ||
return Arrays.stream(ChargerState.values()) | ||
.filter(it -> it.valueString.equals(input)) | ||
.findAny() | ||
.orElse(STATUS_UNKNOWN); | ||
} | ||
|
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.
public static ChargerState from(String input) { | |
return Arrays.stream(ChargerState.values()) | |
.filter(it -> it.valueString.equals(input)) | |
.findAny() | |
.orElse(STATUS_UNKNOWN); | |
} | |
public static ChargerState from(String input) { | |
int num = Integer.parseInt(intput); | |
return Arrays.stream(ChargerState.values()) | |
.filter(it -> it.value == num) | |
.findAny() | |
.orElse(STATUS_UNKNOWN); | |
} | |
이런 방법은 어떤가요 그럼 관리할 필드가 하나 줄어서 나을 것 같아요
@JoinColumn(name = "station_id", referencedColumnName = "station_id", insertable = false, updatable = false, foreignKey = @ForeignKey(value = ConstraintMode.NO_CONSTRAINT)), | ||
@JoinColumn(name = "charger_id", referencedColumnName = "charger_id", insertable = false, updatable = false, foreignKey = @ForeignKey(value = ConstraintMode.NO_CONSTRAINT)) |
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.
@JoinColumn(name = "station_id", referencedColumnName = "station_id", insertable = false, updatable = false, foreignKey = @ForeignKey(value = ConstraintMode.NO_CONSTRAINT)), | |
@JoinColumn(name = "charger_id", referencedColumnName = "charger_id", insertable = false, updatable = false, foreignKey = @ForeignKey(value = ConstraintMode.NO_CONSTRAINT)) | |
@JoinColumns(value = { | |
@JoinColumn(name = "fk_station_id"), | |
@JoinColumn(name = "fk_charger_id") | |
}, foreignKey = @ForeignKey(ConstraintMode.NO_CONSTRAINT)) |
이렇게 하면 한번에 ForeignKey 를 해제할 수 있어요
ZERO(0), | ||
TWELVE(1200); | ||
|
||
private static final List<RequestPeriod> periods = Arrays.stream(values()) | ||
.sorted(Comparator.comparingInt(RequestPeriod::getSection)) | ||
.toList(); | ||
private static final int UNIT = 100; |
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.
해당 클래스 필드,메서드, 상수들의 선언위치가 이상해요 수정부탁드립니다ㅓ
@@ -17,26 +19,45 @@ | |||
@RequiredArgsConstructor | |||
@Slf4j | |||
@Component | |||
public class RestTemplateChargeStationRequester implements ChargeStationRequester { | |||
public class RestTemplateChargeStationRequester implements ChargeStationRequester, ChargerStateRequester { |
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 LocalDateTime parseDateTimeFromString(String input) { | ||
if (input == null || input.length() != 14) { | ||
return null; | ||
} | ||
|
||
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMddHHmmss"); | ||
return LocalDateTime.parse(input, formatter); | ||
} | ||
} |
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.
date time이 이상하게 들어오는 경우가 있었나요?
그렇다면 그 부분들도 parsing 할 수 있도록 만들어야겠네요
|
||
public interface ChargerStatusCustomRepository { | ||
|
||
void saveAll(List<ChargerStateRequest> item); |
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.
파라미터명이 모호하네요 ㅎ
@@ -41,7 +46,7 @@ public StatisticsResponse calculateCongestion(StatisticsRequest statisticsReques | |||
|
|||
private Map<String, List<CongestionInfoResponse>> calculateQuick(List<PeriodicCongestion> congestions, List<Charger> chargers) { | |||
List<String> quickChargerIds = chargers.stream() | |||
.filter(it -> it.getCapacity().intValue() >= 50) | |||
.filter(it -> it.getCapacity().intValue() >= OUTPUT_THRESHOLD) |
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.
이 부분에 capacity가 null 일 경우도 있는데, 그 부분에 대해 처리도 해줘야할 것 같아요. 공공 API에서 옵셔널로 제공되더라고요. 그래서 Charger
클래스 내에 예를 들어 isQuick()
이라는 메서드를 추가하는 것은 어떤가요?
@@ -50,7 +55,7 @@ private Map<String, List<CongestionInfoResponse>> calculateQuick(List<PeriodicCo | |||
|
|||
private Map<String, List<CongestionInfoResponse>> calculateStandard(List<PeriodicCongestion> congestions, List<Charger> chargers) { | |||
List<String> standardChargerIds = chargers.stream() | |||
.filter(it -> it.getCapacity().intValue() < 50) | |||
.filter(it -> it.getCapacity().intValue() < OUTPUT_THRESHOLD) |
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.
ditto
} | ||
|
||
@Test | ||
void 저장() { |
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.
테스트 메서드 이름을 좀 더 길게 적어주세요 ㅜ 무엇을 저장할까요.. ㅜ
@Value("${api.service_key}") | ||
private List<String> serviceKeys; | ||
|
||
public String generateRandomKey() { | ||
int randomNumber = threadLocalRandom.nextInt(START_INDEX, SERVICE_KEY_SIZE); | ||
int randomNumber = threadLocalRandom.nextInt(START_INDEX, serviceKeys.size()); |
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.
근데 이 부분은 yml에 넣어놓은 key가 7개면 0~7 까지의 숫자가 랜덤으로 나오는 것 아닌가요?
그렇게 된다면 7이라는 숫자가 나올 때 out of bound 가 발생할 것 같은데요.
제가 잘못 이해했을 수도 있습니다.
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 LocalDateTime parseDateTimeFromString(String input) { | ||
if (input == null || input.length() != 14) { |
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.
매직넘버 상수처리 해주세요~
|
||
public interface ChargerStateRequester { | ||
|
||
ChargerStateUpdateRequest requestChargerStatusUpdateRequest(int pageNo); |
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.
메서드 명을 requestChargerStatusUpdate
로 바꾸는 건 어떨까요?
public static String generateIdWithCharger(DayOfWeek dayOfWeek, RequestPeriod startTime, Charger charger) { | ||
return generateId(dayOfWeek, startTime, charger.getStationId(), charger.getChargerId()); | ||
} | ||
|
||
public static String generateId(DayOfWeek dayOfWeek, RequestPeriod startTime, String stationId, String chargerId) { | ||
return dayOfWeek.getValue() * 10000L + startTime.getSection() + stationId + chargerId; |
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.
id값이 유니크한 값을 가지면서 데이터처럼 사용되네요👍 이 방법은 추후에 문제점은 없을까요?
OPERATION_SUSPENDED(4), | ||
UNDER_INSPECTION(5), | ||
STATUS_UNKNOWN(9); | ||
COMMUNICATION_ERROR(1, "1"), |
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.
뒤에 valueString
필드를 선언한 이유가 있나요?
메서드에서 Integer.parseInt("...");
로 변경한 후에 비교해도 될 것 같아요~
만약 공공 API에서 null 값 혹은 빈 값이 오는 경우가 있어서 의도 했다면 무시하셔도 됩니다!
private final ChargerStateRequester chargerStateRequester; | ||
private final ChargerStatusCustomRepository chargerStatusCustomRepository; | ||
|
||
@Scheduled(cron = "0 0/7 * * * *") |
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.
작업하시느라 고생하셨습니다 👍
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.
수고하셨습니다
📄 Summary
핵심 과정
혼잡도 저장은 2가지 스텝으로 구분되어있습니다.
1번의 과정에서 ChargerStateUpdateService 의 update 메서드가 schedule 에 등록되어 7분마다 실행됩니다.
없는 충전기가 등록이 된다면, 새롭게 추가되고, 이미 있는 충전기가 등록이 된다면 업데이트 되는 과정을
ON DUPLICATE KEY UPDATE 를 사용해서 구현했습니다.
2번의 과정에서 ChargerStationService 의 calculateCongestion 메서드가 schedule 에 등록되어 1시간마다 실행됩니다.
이 과정에서 현재 충전기가 사용되고 있다면 총 카운트 +1, 사용 카운트 + 1 이 진행됩니다.
충전기가 사용되지 않는다면 총 카운트 +1 만 진행됩니다.
이때 추후 조회 성능을 개선하기 위해서 congestion 도 미리 double 형태로 저장해두도록 만들었습니다.
🙋🏻 More
close #21