Conversation
Jungdahee
left a comment
Jungdahee
left a comment
There was a problem hiding this comment.
안녕하세요 민기님~
먼저 과제 수행하시느라 고생 많으셨습니다.
너무 잘 작성해 주셨는데, 코드를 전반적으로 테스트 해봐야 할 것 같아요!
제가 남긴 코멘트 참고하여 한번 수정해 보시길 바라요~
그리고 추가적으로 commit 단위를 조금 더 나눠서 반영하는 게 좋을 것 같습니다.
지금부터 조금씩 연습하면 아주 좋은 commit 단위와 message를 작성하실 수 있으실 거예요!
고생하셨습니다:)
| private Company companyName; | ||
| private LocalDate dateOfMade; | ||
| private ArrayList<Auth> authMethods = new ArrayList<>(); | ||
| private static int serialNum = 0; |
There was a problem hiding this comment.
static 변수는 일반 변수보다 위에 위치하도록 하는 것이 일반적이예요! 위로 옮겨볼까요~
|
|
||
| public Electronic(String productNo, String modelName, Company companyName, LocalDate dateOfMade, Auth auth) { | ||
| LocalDate dt = LocalDate.now(); | ||
| serialNum++; |
| serialNum++; | ||
| this.modelName = modelName; | ||
| this.productNo = dt.format(DateTimeFormatter.ofPattern("yyMMdd")); | ||
| this.productNo += String.format("%04d", serialNum); |
There was a problem hiding this comment.
데이터의 변환 과정이 잘 보이게 작성해주셨는데, + 연산보다 더 좋은 연산이 있지 않을까요? String에서 + 연산은 속도가 느려요! 지금은 간단한 연산이기에 크게 차이를 못 느낄 수 있지만, 연산이 복잡해질 수록 차이는 확연해져요! 다른 연산을 한번 찾아볼까요?
| private String userPhoneNumber; | ||
| private String userEmail; | ||
| private LocalDate userBirthDate; | ||
| private LocalDate registerTime; |
| this.userPhoneNumber = userPhoneNumber; | ||
| this.userEmail = userEmail; | ||
| this.userBirthDate = userBirthDate; | ||
| registerTime = setRegisterTime(); |
There was a problem hiding this comment.
간단한 연산이니 메소드로 분리할 필요가 없어보여요:) 물론 개인적인 취향이지만요!
|
|
||
| public Electronic findByProductNo(String productNo) { | ||
| for (Electronic electronic : electronicList) { | ||
| if (electronic.getProductNo().equals(productNo)) { |
There was a problem hiding this comment.
이 부분도 stream과 filter를 이용해서 한번 수정해 볼까요~
| return electronic; | ||
| } | ||
| } | ||
| return null; |
| int count = 0; | ||
| for (int i = 0; i < electronicList.length; i++) { | ||
| if (electronicList[i].getCompanyName().equals(company)) { | ||
| groupByCompanyElectronic[count] = electronicList[i]; |
There was a problem hiding this comment.
count 변수에 변화가 없어요! 이렇게 되면 어떤 문제가 발생할까요?
| Electronic[] groupByCompanyElectronic = new Electronic[size]; | ||
| int count = 0; |
There was a problem hiding this comment.
count 변수는 위 코드와 관계가 없는 코드이기 때문에 띄어써주는 게 좋습니다.
| Electronic[] groupByCompanyElectronic = new Electronic[size]; | |
| int count = 0; | |
| Electronic[] groupByCompanyElectronic = new Electronic[size]; | |
| int count = 0; |
| Electronic[] groupByCompanyElectronic = new Electronic[size]; | ||
| int count = 0; | ||
| for (int i = 0; i < electronicList.length; i++) { | ||
| if (electronicList[i].getAuthMethods().equals(authMethod)) { |
There was a problem hiding this comment.
getAuthMethods()의 반환 타입이 []이죠~ equals()로 비교하면 비교가 제대로 될까요?
2 participants
Test 중이긴 하지만 코드 리뷰 부탁드립니다.