-
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
practice03 #15
base: main
Are you sure you want to change the base?
practice03 #15
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.
안녕하세요 찬섭님~
먼저 과제 수행하시느라 고생 많으셨습니다.
전반적으로 잘 수행해주셨는데, commit 단위가 매우 크네요!
다음 번 과제에서는 조금 더 작게 commit을 해주시면 좋을 것 같습니다.
feat, doc, fix 등의 키워드를 활용하여 message를 작성해 보세요!
수고하셨습니다.
package KDTBE5_Java_Assignment3.me.practice.Practice01; | ||
|
||
public enum 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.
한글보다는 영어로 정의하는 게 더 좋을 것 같아요! FINGERPRINT
, PATTERN
, PIN
, FACE
등으로 정의해 보는 건 어떨까요?
import static java.time.LocalDate.now; | ||
|
||
public class Electronic { | ||
public static int registerNumber = 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
변수가 더 위에 정의되어야 한다는 일반적인 관례를 잘 지켜주셨네요! 좋습니다.
private Company companyName; | ||
private String modelName; | ||
private String dateOfMade; | ||
private AuthMethod[] 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.
사소하지만 배열의 형태이니 authMethods로 변경해 보는 건 어떨까요?
private AuthMethod[] authMethod; | ||
|
||
public Electronic(Company companyName, String modelName, String dateOfMade, AuthMethod[] authMethod) { | ||
this.productNo = setProductNo(); |
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 String setProductNo() { | ||
String productNo = now().format(DateTimeFormatter.ofPattern("yyMMdd"))+String.format("%04d",++Electronic.registerNumber); |
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.
정규식을 써서 간단하게 표현해 주신 부분 너무 잘하셨습니다! 다만, String 연산에서 + 는 속도 측면에서 좋은 연산은 아니예요! 다른 연산을 한번 찾아보는 건 어떨까요?
|
||
public class Users { | ||
private static final Users INSTANCE = new Users(); | ||
public 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.
사소한 거지만 한 줄 띄어쓰기!
} | ||
|
||
public static Users getINSTANCE() { | ||
return INSTANCE; |
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.
이것도 하나의 방법이 될 수 있는데, 보통 싱글톤 패턴을 사용할 때 null 인지 체크하고 null 이라면 인스턴스를 생성하여 반환하는 것이 일반적입니다 😃
for (User user: this.users) { | ||
if(user.getUserId().equals(userId))return user; | ||
} | ||
throw new Exception("해당 유저 아이디를 가진 유저는 존재하지 않습니다."); |
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 findByProductNo(String productNo) throws Exception { | ||
for (Electronic electronic:this.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.
Code Convention이 많이 깨져있네요! IDE의 기능을 이용해서 보기 좋게 수정해 주세용~
List<Electronic> electronics = new ArrayList<Electronic>(); | ||
for (Electronic electronic:this.electronicList | ||
) { | ||
if(Arrays.asList(electronic.getAuthMethod()).indexOf(authMethod)>-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.
좋은 아이디어네요! 활용을 잘하셨어요 👋
깊은 복사 부분에서, 새롭게 클래스 객체를 선언하고, 값을 바인딩 해줬으나, 완전 동일한 값에 대하여,
동일한 주소를 보는 얕은 복사가 발생했습니다.
그래서 해당부분 등록 일자나 이런식으로 살짝 바꿔보니 깊은 복사가 됐는데,
혹시 해당 부분은 어떻게 처리하는 것이 맞을까요?