Skip to content

Conversation

@cabbage16
Copy link
Member

1. 구현 여부

요구사항 구현 여부 *

  • 학생은 원서를 제출할 수 있습니다.
  • 학교는 받은 원서를 점수 순으로 다음 정보를 확인할 수 있습니다.
    • 지원자의 이름, 전화번호, 출신 중학교
    • 지원자의 성적, 출결, 가산점
  • 학생은 자신이 제출한 원서의 상태를 확인할 수 있습니다.

제한 사항 구현 여부 *

  • 원서는 한 번만 제출할 수 있습니다.

테스트

  • (진행한 테스트 케이스를 모두 적어주세요)

2. 진행 중 느낀점

과제를 진행하며 한 고민들 *

  • 시험기간과 과제 기간이 겹쳐서 둘 사이의 시간을 적절히 분배해서 하는 것이 고민이었습니다.

발생한 이슈와 해결 방법

  • 학생 객체가 원서를 작성하고 학교 객체가 그 정보를 전달 받아서 합불 여부를 판단해야 하는데, 객체 끼리 정보를 주고받는 것을 구현하는 것이 어려웠습니다.
    해결방법: 원서 객체를 따로 생성해서 각 학생들의 정보를 그 객체에 저장하고 getter와 setter로 정보를 주고 받는 형식으로 해결
  • 학생들의 총 점수를 기준으로 내림차순으로 조회하도록 구현하는 것이 어려웠습니다.
    해결방법: compareTo 메서드를 오버라이딩해서 total값을 기준으로 정렬하도록 했습니다.

질문

  • 메서드의 이름이 너무 길고 가독성이 조금 떨어지는 것 같습니다. 가독성을 해결하는 방법이 없을까요?
  • 처음에 구현할 때는 원서 객체를 따로 만드는 방식을 생각하지 못했는데, 원서 객체를 만들지 않고도 학생의 원서 정보를 학교가 받도록 구현하는 방법은 없을까요?

-학교 객체를 생성
-학생 객체를 생성하고 생성자를 정의
-학생이 원서를 제출하고 재제출시 정상적으로 처리되지 않도록 기능 추가
-학생의 원서 정보를 학교가 원활하게 받기 위해서 각각의 학생마다 자신의
원서 객체를 생성하고 그 원서 객체에 최종적인 정보를 담음.
-학교 객체가 학생을 점수 순으로 조회하는 기능 추가.
-학교 객체가 기준을 둬서 기준 이상의 학생만 합격 시키도록 기능 추가.
-compareTo메서드 오버라이딩으로 total을 기준으로 정렬하는 기능 추가.(주석 참고)
-학생이 자신의 원서 상태를 조회해서 합격, 불합격 여부 확인하는 기능
추가.
@cabbage16 cabbage16 requested a review from a team as a code owner June 26, 2023 13:00
@gimhanul
Copy link
Member

🙌 @cabbage16 님 안녕하세요

밤돌이로 백엔드 개발 팀입니다.

먼저, 과제 전형 기간 동안 수고하셨습니다!
과제를 수행하시면서 많이 배우셨길 바랍니다.

과제를 준비하면서 얻어가셨길 바랐던 것은

1. 자기주도적으로 공부하기
2. 객체지향적으로 설계하기
3. 깃허브 사용법 익히기

인데요, 다들 어느정도 얻어가신 것 같아 뿌듯하네요.

과제 전형 점수 안내드립니다.

자세한 감점 사유가 궁금하면 따로 연락주세요.

코드 가독성 실행 학구열 성실성 테스트 총점
28 28 30 10 0 96

총점 96점으로, 1차 과제 전형에서 합격하셨습니다.
오늘 안으로 면접 일정을 잡아 개인적으로 연락드리겠습니다!

감사합니다.

-명명 규칙에 따라 일부 객체 이름을 변경함
-필요없는 주석을 삭제함
-코드 가독성을 높이기 위해 공백 추가
Copy link
Member

@gimhanul gimhanul left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~
리뷰 참고해서 반영하셔도 되고, 그대로 머지하셔도 됩니다!

private int intGrade;
private boolean pass;

public void write(String name, String phoneNumber, String fromMS, char grade, int attendance, int plusPoint){ //원서를 작성하는 함수
Copy link
Member

Choose a reason for hiding this comment

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

생성자랑 역할이 겹치는 거 같아요!
생성자로 사용하는 게 좋을 듯 하네요

Comment on lines 58 to 60
if (ApplicationForm.total < total){ return 1; } //ApplicationForm.total을 기준으로 정렬하도록 함
else if (ApplicationForm.total > total){ return -1; }
return 0;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (ApplicationForm.total < total){ return 1; } //ApplicationForm.total을 기준으로 정렬하도록 함
else if (ApplicationForm.total > total){ return -1; }
return 0;
return total - ApplicationForm.total;

간결하게 표현할 수 있어용

Copy link
Member

Choose a reason for hiding this comment

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

그리고 이렇게 되면 굳이 Comparable interface를 implments하지 않아도 lambda 식으로 더 간결하게 표현할 수 있어요.
lambda 키워드로 한 번 알아보시면 좋을 거 같아ㅛㅇ

}

@Override
public int compareTo(ApplicationForm ApplicationForm){ //객체의 compareTo메서드를 오버라이딩하여 ApplicationForm객체만의 정렬 조건을 재정의
Copy link
Member

Choose a reason for hiding this comment

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

자바에서는 클래스에는 파스칼 케이스를, 변수 이름에는 카멜 케이스를 사용하빈다
저렇게 사용하면 자칫하다간 ApplicationForm에 static 변수라도 있으면 그 변수로 쓰게 되겠네요

파스칼 케이스, 카멜 케이스에 대해서 한 번 알아보세용
그리고 변수 이름은 꼭!! 수정해주세요

Comment on lines 52 to 54
public void setPass(boolean pass){
this.pass = pass;
}
Copy link
Member

Choose a reason for hiding this comment

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

setter 사용은 지양하는 게 좋습니다
관련해서 검색해보세용

// TODO-3 이밤돌 학생 원서 재제출
// 이밤돌 학생이 부산소프트웨어마이스터고등학교에 낼 원서를 작성합니다.

leebamdol.setApplicationForm('A', 100, 1);
Copy link
Member

Choose a reason for hiding this comment

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

A, B 같은 건 그냥 String으로 넣게 되면 사용자가 다른 값을 넣을수도 있어요
그래서 enum으로 관리해주는 게 좋습니다!!

enumeration으로 검색해보세용


leebamdol.setApplicationForm('A', 100, 1);
// 이밤돌 학생이 원서를 제출합니다.
geumgomdol.submitApplicationForm(bssm);
Copy link
Member

Choose a reason for hiding this comment

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

감동적인 코드...
이렇게 짠 친구 많지 않아요


public class School {
private final String schoolName;
private final ArrayList<ApplicationForm> applicationForms = new 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.

final에 대해서 잘 아시나용?
final을 썼을 때와 안 썼을 때의 차이를 설명해주세용

Comment on lines 30 to 33
for(ApplicationForm applicationForm : applicationForms){
applicationForm.setPass(applicationForm.getTotal() >= 80);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

stream이라는 게 있는데 이걸 사용하면 for문을 쓰지 않아도 간편하게 사용할 수 있어용
관련된 내용 stream 키워드로 검색해서 알압소요

private final String fromMS;
private final String phoneNumber;
private final ApplicationForm applicationForm = new ApplicationForm(); //학생 객체가 만들어질 때 마다 각자의 원서 객체를 생성함
private int count = 1; //모든 학생들은 한 번만 원서를 제출할 수 있음
Copy link
Member

Choose a reason for hiding this comment

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

이런 변수는 count보다는 제출 횟수 정도의 이름으로 나타내는 게 좋을 거 같아요
변수 이름은 개발자 본인의 간편함보다는 다른 사람이 읽었을 떄 잘 읽을 수 있게 짓는 게 좋아요

Copy link
Member

Choose a reason for hiding this comment

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

그리고 냈다 / 안 냈다 의 상태밖에 없으니까 boolean으로 관리하는 것도 좋겠네요

@gimhanul gimhanul changed the base branch from main to jaehyeon July 1, 2023 16:30
- 이밤돌 학생의 원서 재제출이 금곰돌 학생으로 되어있는 오류 수정
- JAVA 17 -> 11 사용
- switch 문 대신 Grade.java enum 클래스를 활용하여 성적을 변환
- compareTo 메서드에서 매개변수 이름 ApplicationForm -> applicationForm 카멜 케이스 사용
- 패키지 이름 Main -> main
- 일부 주석 오타 수정
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants