2024 08 02 TIL 쓰레기코드 양산하는 사람
Java 프로그래밍 기초 팀 과제를 시작하고, 내가 맡은 역할 중에 어떤 수강생의 과목별 평균 등급을 조회해야 하는 기능을 만들어야 하는 게 있다. 우리는 score 객체 안에 score id, 학생 id, 과목id, 회차, 점수 , 등급을 필드로 보관하는데, 여기서 학생id, 과목id를 찾아서 그 학생의 그 과목 점수들을 관리하는 방식이다.
그래서 일단 코드를 짜다가, 중간에 아까 만들었던 코드를 수정해야 할 일이 생겼는데, 그냥 쳐다만 봐도 수정 하기 싫게 짜여있어서 놀랐다. 짠지 얼마 안 된 코드인데도, 너무 개판이었다.
public static char filterAndPrintAverageGradeByStudentandSubject(String studentId, String subjectId, int subjecttype) {
List<Score> filteredScore = scoreStore.stream()
.filter(score -> studentId.equals(score.getStudentId()) && subjectId.equals(score.getSubjectId()))
.collect(Collectors.toList());
double result = 0;
double average = 0;
for (Score score : filteredScore) {
result+=score.getScore();
}
average = result/10;
return getGrade(average,subjecttype);
}
public static void studentAverageGradeBySubject(Student student) {
String stid = student.getStudentId();
System.out.println( student.getStudentName()+"님의 과목별 평균 등급은 다음과 같습니다..");
/* *
필수과목, 선택과목 등급 산정 기준이 다름
필수과목 평균 + 선택과목 평균 내서 그냥 갖다 붙여버리기
*/
List<Subject> subjectList = student.getSubjectList();
List<Subject> mandatorySubjectList = subjectList.stream()
.filter(s -> "MANDATORY".equals(s.getSubjectType()))
.collect(Collectors.toList());
List<Subject> choiceSubjectList = subjectList.stream()
.filter(s -> "CHOICE".equals(s.getSubjectType()))
.collect(Collectors.toList());
for (Subject subject : mandatorySubjectList) {
String sName = subject.getSubjectName();
String sId = subject.getSubjectId();
char grade = filterAndPrintAverageGradeByStudentandSubject(stid, sId,1);
System.out.printf("[(필) %s : %c ]\n", sName,grade);
}
for (Subject subject : choiceSubjectList) {
String sname = subject.getSubjectName();
String sid = subject.getSubjectId();
char grade = filterAndPrintAverageGradeByStudentandSubject(stid,sid,2);
System.out.printf("[(선) %s : %c ]\n",sname,grade);
}
}
private static char getGrade(double result, int subjecttype) {
//subjecttype이 1인 경우는 필수과목, 2인 경우는 선택과목
char Grade = 'N';
if (subjecttype == 1) {
if (result >= 95) {
Grade = 'A';
} else if (result >= 90) {
Grade = 'B';
} else if (result >= 80) {
Grade = 'C';
} else if (result >= 70) {
Grade = 'D';
} else if (result >= 60) {
Grade = 'F';
} else {
Grade = 'N';
}
} else if (subjecttype == 2) {
if (result >= 90) {
Grade = 'A';
} else if (result >= 80) {
Grade = 'B';
} else if (result >= 70) {
Grade = 'C';
} else if (result >= 60) {
Grade = 'D';
} else if (result >= 50) {
Grade = 'F';
} else {
Grade = 'N';
}
} return Grade;
}그냥 째려보고 있자니 시간은 가고, 진척은 없어서 일단 각 메서드들이 하는 일을 정리했다.
각 메서드 로직
filterAndPrintAverageGradeByStudentandSubject (String studentId, String subjectId, int subjecttype) >> return char
1) 학생id, 과목 id와 일치하는 스코어를 받아와서 리스트에 갖다쳐박는다.
2) 스코어 리스트 순회하면서 전부 더해버림
3) 평균값 내서 등급 변환기서 바꿔옴 (average, subjecttype)
studentAverageGradeBySubject (Student student) >> void
수강생이 듣는 과목 전체 리스트 싹 긁어옴
"MANDATORY"일 경우 필수선택과목 리스트 만들어서 넣음
"CHOICE"일 경우 선택과목 리스트 만들어서 넣음
필수과목 리스트에서 과목 이름, 과목 ID 긁어와서 평균낸 등급 출력
[(필) : 평균등급]
선택과목 리스트에서 과목 이름, 과목 ID 긁어와서 평균낸 등급 출력
[(선) : 평균등급]
이렇게 보니 문제점이 비교적 한 눈에 들어오기 시작했는데, 과목 리스트 만들 때 "과목타입"만 다른 서로 같은 코드를 쓰기 때문에, 비효율적이고 가독성도 안 좋고 코드에서 음식물쓰레기 냄새도 나는 거 같다. 지금이야 "필수 과목", "선택 과목"만 있어서 두 번만 반복되지만
"전공 필수" "전공 선택" "교양 필수" "교양 선택" "자유 선택" 이런 식으로 더 쪼개지는 순간 같은 코드를 다섯 번이나 써야한다. 끔찍하다. 때문에 일단 같은 기능을 하고있는 것들부터 메서드화 시켜서 밖으로 빼내보기로 했다.
// 수강생 과목별 평균 등급 조회 코어 메서드
private static void studentAverageGradeBySubject(Student student) {
System.out.println( student.getStudentName()+"님의 과목별 평균 등급은 다음과 같습니다..");
/* *
필수과목, 선택과목 등급 산정 기준이 다름
필수과목 평균 + 선택과목 평균 내서 붙여버리기
*/
//여기서 동작하지는 않지만, 같은 student 객체를 쓰기 위해 여기에 넣었습니다.
listStudentSubjectByType(student, "MANDATORY");
listStudentSubjectByType(student, "CHOICE");
printAVGGradebySubject(student,"MANDATORY");
printAVGGradebySubject(student,"CHOICE");
}
//수강생의 해당 과목 평균 등급을 반환
private static char filterAndReturnAverageGradeByStudentandSubject(String studentId, String subjectId, String subjectTypeLabel) {
List<Score> filteredScore = scoreStore.stream()
.filter(score -> studentId.equals(score.getStudentId()) && subjectId.equals(score.getSubjectId()))
.toList();
double result = 0;
double average = 0;
for (Score score : filteredScore) {
result+=score.getScore();
}
average = result/10;
return getGrade(average,subjectTypeLabel);
}
//수강생이 듣는 과목을 (전공,선택)에 따라 리스트로 반환
private static List<Subject> listStudentSubjectByType(Student student , String type) {
List<Subject> subjectList = student.getSubjectListTypeSubject();
return subjectList.stream()
.filter(s -> type.equals(s.getSubjectType())).toList();
}
//수강생이 듣는 과목별로 평균등급을 출력해주는 메서드
private static void printAVGGradebySubject (Student student, String subjectTypeLabel) {
for (Subject subject : listStudentSubjectByType(student, subjectTypeLabel)) {
String stid = student.getStudentId();
String sName = subject.getSubjectName();
String sId = subject.getSubjectId();
String simpleSubjectType;
if (Objects.equals(subjectTypeLabel, "MANDATORY")) {simpleSubjectType = "(필)";} else {simpleSubjectType = "(선)";}
char grade = filterAndReturnAverageGradeByStudentandSubject(stid, sId, subjectTypeLabel);
System.out.printf("[ %s %s : %c ]\n",simpleSubjectType,sName,grade);
}
}
//점수를 (전공,선택)에 따라 등급으로 변환해주는 메서드
private static char getGrade(double result, String subjectTypeLabel) {
//subjecttype이 1인 경우는 필수과목, 2인 경우는 선택과목
char Grade = 'N';
if (subjectTypeLabel == "MANDATORY") {
if (result >= 95) {
Grade = 'A';
} else if (result >= 90) {
Grade = 'B';
} else if (result >= 80) {
Grade = 'C';
} else if (result >= 70) {
Grade = 'D';
} else if (result >= 60) {
Grade = 'F';
} else {
Grade = 'N';
}
} else if (subjectTypeLabel == "CHOICE") {
if (result >= 90) {
Grade = 'A';
} else if (result >= 80) {
Grade = 'B';
} else if (result >= 70) {
Grade = 'C';
} else if (result >= 60) {
Grade = 'D';
} else if (result >= 50) {
Grade = 'F';
} else {
Grade = 'N';
}
} return Grade;
}
확실히 기능들을 쪼개고 나니 아까 전 코드보단 수정하기도 편해지고, 다음에 또 꺼내쓰기에도 좋아진 듯하다. 사실 내 코드를 수정하려는데 너무 안 읽혀서 째려보다가 시간만 낭비한 적은 이번이 처음은 아니라서, 리팩터링의 중요성을 느끼게 되었고, 생각난 김에 바로 추천받은 리팩터링 책을 주문했다.
이번 주말은 물론이고 앞으로 개발을 하면서 계속 틈틈히 읽어가면서 점점 좋은 코드를 작성하려고 노력해야겠다....