-
Notifications
You must be signed in to change notification settings - Fork 0
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
Java Assignment3 upload by HeehyunKim #33
base: main
Are you sure you want to change the base?
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.
안녕하세요 희현님~
과제 진행하신다고 고생많으셨습니다.
깊은 복사의 개념에 대해 확실히 집고가셨으면 좋겠습니다. 메모리 주소값과 연관있는 굉장히 중요한 지식이에요!
다음번에 과제 제출해주실때는 제출전에 main
함수에서 실행시켜보고 요구사항을 모두 만족하는지 확인해보면 좋을 것 같아요!
동일한 코드가 반복된다면 별도의 로직으로 분리하거나 더 나은 방향을 고민해보세요!~😀
코드를 작성하실 때 코드 스타일에 대해서도 조금 신경 써주셨으면 좋겠습니다. 인텔리제이의 자동 정렬 기능을 사용해보세요~
메서드, 변수들의 위치, 줄바꿈 등등
깃 커밋의 단위를 좀 더 잘개 쪼개면 좋을 것 같아요!
ex)
- feat: User 클래스 생성 및 필드 작성
- feat: 생성자 생성
- feat: 싱글톤 패턴 구현
- feat: 회원 조회 메서드 구현
- refactor: 회원 조회 메서드 로직 분리
요렇게요!!
커밋 내역만으로 어떤 작업을 했는지 알 수 있다면 추후 협업 과정에서 큰 도움을 받을 수 있을 거에요~!
다음 과제에서는 요구사항을 좀 더 꼼꼼히 읽어보셨으면 좋겠습니다!
그리고 pr을 올리실 때 본인만의 템플릿을 가지고 계시면 좋을 것 같습니다.
ex)
- 내가 중점적으로 생각한 부분
- 작업 내용
- 궁금한 점
- 함께 이야기하고 싶은 부분
private static int numProducts = 0; | ||
|
||
public Electronic(String modelName, CompanyName companyName, LocalDate dateOfMade, AuthMethod[] authMethods){ | ||
this.modelName = modelName; |
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 CompanyName companyName; | ||
private LocalDate dateOfMade; | ||
private AuthMethod[] authMethods; | ||
private static int numProducts = 0; |
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.
일반적으로 static
변수가 최상단에 위치합니다~
this.companyName = companyName; | ||
this.dateOfMade = dateOfMade; | ||
this.authMethods = authMethods; | ||
this.productNo = ProductNo(); |
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.
메서드이기 때문에 카멜케이스를 적용시켜야할 것 같고, 좀 더 적절한 네이밍은 없을까요?
generateProductNo()
?
numProducts++; | ||
} | ||
|
||
private String ProductNo(){ |
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
메서드로 분리해주셨네요 👍🏻
|
||
private String ProductNo(){ | ||
String date = LocalDate.now().format(DateTimeFormatter.ofPattern("yyMMdd")); | ||
date += String.format("%04d", numProducts+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.
+1
이 필요한가요??
|
||
public class Users { | ||
private User[] userList; | ||
private static Users instance = new Users(); |
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.
static
변수, 일반 변수, 생성자, public
메서드, private
메서드 순으로 코드를 작성해주세요.
사소하지만 정말 중요하답니다.
} | ||
|
||
public User findByUserId(String userId){ | ||
for (User user : userList){ |
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.
Stream
으로 해당 동작을 수행하게 해볼까요?
userCopy.setUserBirthDate(user.getUserBirthDate()); | ||
userCopy.setUserPhoneNumber(user.getUserPhoneNumber()); | ||
userCopy.setRegisterTime(user.getRegisterTime()); | ||
userCopy.setElectronicDevices(user.getElectronicDevices()); |
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 Electronic[] groupByCompanyName(Electronic.CompanyName company){ | ||
int count=0; | ||
for(Electronic electronic : electronicList){ |
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.
아래와 코드가 중복되는데요~ 중복을 줄이는 방법은 없을까요?
Stream
을 사용하면 한번에 다 처리할 수 있을 것 같습니다.
public Electronic[] groupByAuthMethod(Practice01.Electronic.AuthMethod authMethod){ | ||
int count=0; | ||
for(Electronic electronic : electronicList){ | ||
if(electronic.getAuthMethods().equals(authMethod)){ |
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.
getAuthMethods()
의 반환형은 배열인데 객체와 값을 비교하는게 맞을까요?
No description provided.