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 HoyunJung #30

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

Conversation

ho30archive
Copy link

클린한 코드인지 확인 부탁드립니다! Electronics.class의 groupByAuthMethod와 groupByCompanyName가 올바르게 작성된건지 궁금합니다.

if(company == null) return null;
if(allElectronics == null) return null;

Electronic[] groupByCN = new Electronic[allElectronics.size()];

Choose a reason for hiding this comment

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

몇개인지 모르는 상황에서는 저라면 컬렉션(ArrayList)을 사용한 뒤 loop 후 반복문이 끝난 뒤 배열로 바꿔줄 것 같아요. 필터링된 리스틀만 리턴받는 메소드에서 리턴 받은 후 외부에서 해당 배열의 길이를 사용하는 로직이 있게되면 로직이 꼬이게 되거든요

if(authMethod == null) return null;
if(allElectronics == null) return null;

Electronic[] groupByAM = new Electronic[allElectronics.size()];

Choose a reason for hiding this comment

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

여기도 컬랙션 사용 후 배열로 바꿔주는게 나을것 같아요

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