Skip to content
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 MiyeonLee #27

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

miyounlee
Copy link

코드리뷰 빡세게 부탁드립니다!

Copy link

@lalwr lalwr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과제하느라 고생하셨습니다 👍

작성한 코드가 잘 작동하는지 테스트 코드를 작성해보는건 어떨까요?

}

public Electronic(String modelName, Company companyName, String dateOfMade, AuthMethod[] authMethod) {
serialNum++;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중복 코드를 줄여보세요.

private String userEmail;
private int userBirthDate;
private String[] electronicDevices;
public LocalTime registerTime;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과제는 LocalDate 를 사용하라고 나와있는데 LocalTime 를 사용하고 있네요.

Copy link
Author

@miyounlee miyounlee Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Electronic 클래스의 productNo은 LocalTime을 사용하라고 언급되어 있는데
User클래스의 registerTime에 대해선 언급이 없는 것 같습니다!

근데 멘토님 코멘트 보고 생각해보니, 회원정보 등록 시간에 대한 필드니까 날짜와 시간이 같이 출력되도록 하는 것이 좋을 것 같습니다. 참고해서 수정해보겠습니다.🤓

return instance;
}

User findByUserId(String userId) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

접근 제한자가 없는 이유가 있을까요?


User findByUserId(String userId) {
for(User user : userList) {
if(user.getUserId().equals(userId))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

equals 👍


User copy(User user) {
User userCopy = new User(user.getUserId(), user.getUserPassword(), user.getUserPhoneNumber()
, user.getUserEmail(), user.getUserBirthDate(), user.getElectronicDevices());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.getElectronicDevices() 의 데이터를 변경하면 어떻게 될까요?

}

User copy(User user) {
User userCopy = new User(user.getUserId(), user.getUserPassword(), user.getUserPhoneNumber()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registerTime 도 복사해야 하지 않을까요?

}

private Electronics getInstance() {
if (!(instance == null)) return instance = new Electronics();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null 일때 생성해야 하지 않을까요?

Suggested change
if (!(instance == null)) return instance = new Electronics();
if ((instance == null)) return instance = new Electronics();


int j = 0;
for (int i = 0; i < electronicList.length; i++) {
if (electronicList[i].getAuthMethod().equals(authMethod)){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.getAuthMethod() 는 배열이지 않나요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants