[ADD] Mingi Park's 답변 #35
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jungdahee
reviewed
Apr 27, 2023
| 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 변수에 변화가 없어요! 이렇게 되면 어떤 문제가 발생할까요?
Comment on lines
+107
to
+108
| Electronic[] groupByCompanyElectronic = new Electronic[size]; | ||
| int count = 0; |
There was a problem hiding this comment.
count 변수는 위 코드와 관계가 없는 코드이기 때문에 띄어써주는 게 좋습니다.
Suggested change
| 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()로 비교하면 비교가 제대로 될까요?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test 중이긴 하지만 코드 리뷰 부탁드립니다.