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 JeonghoSong #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sdfgx123
Copy link

@sdfgx123 sdfgx123 commented Apr 17, 2023

안녕하세요, 자바 강의 3주차 과제 제출합니다.

Electronics 클래스 같은 경우 ArrayList를 가지고 로직을 처리하고, 다시 객체 배열로 바꿔서 리턴하는 방식으로 진행했습니다.
이렇게 하면 코드가 비효율적인가요??

Electronic 클래스 안에 enum을 만들려고 했는데, groupByName 메서드와 groupByAuthMethod 메서드에서 파라미터를
받을 때 에러가 계속 나던데 같은 패키지 안에 없어서 그런건가요??

코드 리뷰 부탁드립니다. 감사합니다.

@sdfgx123 sdfgx123 requested a review from ecsimsw April 17, 2023 12:54
@sdfgx123 sdfgx123 changed the title java assignment third uploaded Java Assignment3 upload by JeonghoSong Apr 17, 2023
Comment on lines +63 to +84
public Electronic[] groupByCompanyName(CompanyName company) {
List<Electronic> res = new ArrayList<>();
for (Electronic e : electronicList) {
if (e.getCompanyName()==company) res.add(e);
}
return res.toArray(new Electronic[0]);
}

public Electronic[] groupByAuthMethod(AuthMethod authMethod) {
List<Electronic> electronicListByAuthMethod = new ArrayList<>();
for (Electronic e : electronicList) {
AuthMethod[] authMethods = e.getAuthMethods();
for (AuthMethod method : authMethods) {
if (method==authMethod) {
electronicListByAuthMethod.add(e);
break;
}
}
}
Electronic[] res = new Electronic[electronicListByAuthMethod.size()];
return electronicListByAuthMethod.toArray(res);
}
Copy link
Author

@sdfgx123 sdfgx123 Apr 17, 2023

Choose a reason for hiding this comment

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

1. 메서드 파라미터 에러

위쪽에 있는 groupByCompanyName 메서드와, 아래쪽에 있는 groupByAuthMethod 메서드를 호출하면서 파라미터를 enum으로 받으려고 했는데 에러가 났습니다.

에러 내용은 CompanyName과 AuthMethod Enum을 따로 새로운 클래스로 만들라는 것이었습니다. practice01 패키지 안에 새로 enum 클래스를 만들어서 해결했습니다.

제가 궁금한 건 Electronic 클래스 안에서 enum을 선언하면 왜 다른 메서드에서 파라미터를 받을 때 에러가 발생하는 건가요??

2. ArrayList를 다시 배열로 바꿀 경우의 문제

두 메서드의 경우 ArrayList를 가지고 로직을 처리하고, 다시 객체 배열로 바꿔서 리턴하는 방식으로 진행했습니다. 물론 과제에서 리턴을 객체 배열로 하라고 해서 했던 것이었는데, 이 메서드의 방식이 비효율적인가요? 아니면 원래 과제의 목적은 리스트 인터페이스를 쓰는 게 아니었는데 제가 임의로 써버려서 그런 걸까요?

Copy link
Member

Choose a reason for hiding this comment

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

  1. Import를 제대로 하셨나요?
    import를 import (앞 경로).practice01. Electronic;으로 하시고 사용하실땐 'Electronic.XXX'으로 하셔야 할거예요.
  2. 잘하셨습니다. 좋은 방식이라고 생각합니다.

Copy link
Member

@ecsimsw ecsimsw left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.

이미 잘하셨는데 한번 다시 봤으면 하는 것들을 커멘트로 남겨두었어요.
다른 것은 괜찮아도 Electronic 안에 main으로 테스트하는 코드는 꼭 확인해주시고, enum 질문도 import 다시 해보셨으면 좋겠습니다.

}



Copy link
Member

Choose a reason for hiding this comment

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

여기 많이 띄어져 있네요 ㅎㅎ




public static void main(String[] args) {
Copy link
Member

Choose a reason for hiding this comment

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

오잉? 이건 지우는게 좋을 것 같아요.
혼자 테스트 하는 용도라도 진짜 프로젝트 main문에서 해보실까요?

Electronic class에서 하시면 안될 것 같아요!

result = 31 * result + userEmail.hashCode();
result = 31 * result + Integer.hashCode(userBirthDate);
result = 31 * result + electronicDevices.hashCode();
result = 31 * result + registerTime.hashCode();
Copy link
Member

Choose a reason for hiding this comment

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

오홍 여기선 위처럼 Objects.hash()를 사용하지 않은 이유가 있나요?

int userPhoneNumber = user.getUserPhoneNumber();
String userEmail = user.getUserEmail();
int userBirthDate = user.getUserBirthDate();
ArrayList<String> electronicDevices = new ArrayList<>(user.getElectronicDevices());
Copy link
Member

Choose a reason for hiding this comment

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

문제!!!
여기서 아래처럼 electronicDevices를 넘겨주면 안될까요?

 ArrayList<String> electronicDevices = user.getElectronicDevices();

for (AuthMethod method : authMethods) {
if (method==authMethod) {
electronicListByAuthMethod.add(e);
break;
Copy link
Member

Choose a reason for hiding this comment

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

break 잘하셨네요!!

Comment on lines +63 to +84
public Electronic[] groupByCompanyName(CompanyName company) {
List<Electronic> res = new ArrayList<>();
for (Electronic e : electronicList) {
if (e.getCompanyName()==company) res.add(e);
}
return res.toArray(new Electronic[0]);
}

public Electronic[] groupByAuthMethod(AuthMethod authMethod) {
List<Electronic> electronicListByAuthMethod = new ArrayList<>();
for (Electronic e : electronicList) {
AuthMethod[] authMethods = e.getAuthMethods();
for (AuthMethod method : authMethods) {
if (method==authMethod) {
electronicListByAuthMethod.add(e);
break;
}
}
}
Electronic[] res = new Electronic[electronicListByAuthMethod.size()];
return electronicListByAuthMethod.toArray(res);
}
Copy link
Member

Choose a reason for hiding this comment

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

  1. Import를 제대로 하셨나요?
    import를 import (앞 경로).practice01. Electronic;으로 하시고 사용하실땐 'Electronic.XXX'으로 하셔야 할거예요.
  2. 잘하셨습니다. 좋은 방식이라고 생각합니다.

import java.util.Arrays;

public class Users {
private static Users instance = null;
Copy link
Member

Choose a reason for hiding this comment

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

제 개인적으로 객체명으로 instance는 고치고 싶습니다.
모든 객체는 인스턴스 되었다면 instance이니 메서드 명으로 method, class 명으로 class 처럼 보이네요. 게다가 초기값을 null로 주니 instance도 아닌거 같고..

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