핵심교양 졸업 계산 기능 구현 #161

5uhwann · 2023-08-11T18:49:43Z

Issue

Close #160

✅ 작업 내용

핵심교양 세부 졸업 결과 반환 기능 구현
ICT 학부 학생 SW프로그래밍 입문 수강 시 자유학점 적용 구현
4차산업혁명시대의예술, 문화리터러시와창의적스토리텔링 2022 1학기 수강 시 일반교양 학점 적용 구현

codecov · 2023-08-11T18:52:24Z

Codecov Report

Merging #161 (100c8ae) into develop (6706d9b) will increase coverage by 5.83%.
The diff coverage is 96.33%.

@@              Coverage Diff              @@
##             develop     #161      +/-   ##
=============================================
+ Coverage      65.56%   71.40%   +5.83%     
- Complexity       105      142      +37     
=============================================
  Files             28       32       +4     
  Lines            424      514      +90     
  Branches           7       11       +4     
=============================================
+ Hits             278      367      +89     
+ Misses           138      136       -2     
- Partials           8       11       +3

Files Changed	Coverage Δ
.../graduation/domain/model/DetailCategoryResult.java	`94.54% <90.47%> (-2.83%)`	⬇️
.../coreculture/CoreCultureDetailCategoryManager.java	`95.00% <95.00%> (ø)`
...demicalculture/BusinessBasicAcademicalManager.java	`83.87% <100.00%> (ø)`
...ademicalculture/DefaultBasicAcademicalManager.java	`94.11% <100.00%> (ø)`
...icalculture/SocialScienceBasicAcademicManager.java	`93.10% <100.00%> (+0.24%)`	⬆️
...monculture/CommonCultureDetailCategoryManager.java	`100.00% <100.00%> (ø)`
...vice/coreculture/CoreCultureGraduationManager.java	`100.00% <100.00%> (ø)`
...jigraduatebe/lecture/domain/model/CoreCulture.java	`100.00% <100.00%> (ø)`
...tebe/lecture/domain/model/CoreCultureCategory.java	`100.00% <100.00%> (ø)`
...yongjigraduatebe/lecture/domain/model/Lecture.java	`76.66% <100.00%> (+2.59%)`	⬆️

... and 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6706d9b...100c8ae. Read the comment docs.

stophwan

전체적으로 final 변수를 사용함에 있어 오히려 코드 가독성과 응집도 측면에서 오히려 떨어지는 느낌이 들어!
우리 서비스 특성상 DetailCategoryResult의 모든 필드를 final로 한다면 차라리 현재 도메인서비스에 필드를 가지고 매핑하는게 차리리 맞고, 아니면 변경가능한 부분은 메서드를 통해 수정하는게 맞는것같아!
이에 대해 여러가지 코멘트들을 달았는데 수환이 생각이 궁금하네.

stophwan · 2023-08-12T03:31:08Z

...ain/java/com/plzgraduate/myongjigraduatebe/graduation/domain/model/DetailCategoryResult.java

 	public static DetailCategoryResult create(String detailCategoryName, boolean isSatisfiedMandatory,
-		int totalCredits) {
+		int totalCredits, int normalLeftCredit, int freeElectiveLeftCredit) {
 		return DetailCategoryResult.builder()
 			.detailCategoryName(detailCategoryName)
 			.isCompleted(false)
 			.isSatisfiedMandatory(isSatisfiedMandatory)
 			.totalCredits(totalCredits)
 			.takenCredits(0)
+			.normalLeftCredit(normalLeftCredit)
+			.freeElectiveLeftCredit(freeElectiveLeftCredit)
 			.build();
 	}


필드를 final로 두기 위해 isSatisfiedMandatory, normalLectCredit, freeElectiveLeftCredit를 전부 주입을 받는데, final을 안쓰고 따로 메서드를 통해 값을 변경해도 되지 않을까? 하는게 내 생각이야!
final 변수를 사용하는것이 안정성 면에서는 무조건 좋지만 그것을 지키겠다고 너무 잃는게 많은 느낌? SW프로그래밍입문 같은 경우같은 것을 제외하고는 leftCredit은 taken - total인데, final을 지키기 위해서 모든 클래스에 getLeftCredit을 넣어야하니까. 사실 직접 leftCredit을 증가시키는 메서드를 사용해도 되는데.
또 특정 케이스말고 isSatisfiedMandatory, normalLectCredit, freeElectiveLeftCredit 필드를 필요로 하지않는데
그리고 이렇게 되면 모든 클래스에서 이 값을 주입시켜야하는 메소드나 추가적인 작업이 들어가야하고!

leftCredit을 외부에서 계산하고 주입하는 이유는 각 졸업 카테고리 별로 일괄적으로 일반교양 혹은 자유선택으로 taken-totalCredit이 넘어가는 것이 아니라 같은 졸업 카테고리 안에서도 세부 카테고리에 따라 일반교양, 자유선택으로 넘어가는 과목이 달라서 이런 방법을 택했습니다.
만약 DetailCategoryResult안에서 해당 로직을 처리하면 DetailCatgoryResult에 특정 예외를 처리하는 로직이 들어가는데 좋지 않은 방향이라 생각했습니다.

모든 클래스에 getLeftCredit이 중복되어 있다는 것은 원래는 DetailCategoryResult에 있어야한라고 생각해.
지금 좀 생각을 해봤는데 차라리 SW프로그래밍입문처럼 일반교양으로 넘어거나는 경우는 융합소프트웨어학부의 경우는 일반교양으로 넘어가가게 graduationLectures에서 제외시키면 되는거 아닌가?
그리고 카테고리별로 일반고양, 자유선택으로 넘어가는 부분은 DetailCategoryResult의 상위 클래스인 DetailGraduationResult에서 DetailCategoryResult의 getLeftCredit 메서드를 호출하는 것으로 해결하는게 나아보이는데 어떻게 생각해?

stophwan · 2023-08-12T03:33:12Z

...duatebe/graduation/domain/service/basicacademicalculture/BusinessBasicAcademicalManager.java

+	private int getNormalLeftCredit(Set<Lecture> taken, int categoryTotalCredit) {
+		int totalTakenCredit = taken.stream()
+			.mapToInt(Lecture::getCredit)
+			.sum();
+		int normalLeftCredit = totalTakenCredit - categoryTotalCredit;
+		return Math.max(normalLeftCredit, 0);
+	}
 }


마찬가지로 굳이 여기서 계산을 하는것보다는 DetailCategoryResult 안에서 계산하는것이 더 맞지 않을까? 하는 생각!
SW프로그래밍입문과 같은 경우를 제외하면 모든 leftCredit은 기본적으로 takenLecture - totalCredit이니!

stophwan · 2023-08-12T03:55:34Z

...jigraduatebe/graduation/domain/service/commonculture/CommonCultureDetailCategoryManager.java

+		int leftCredit = getLeftCredit(taken, category.getTotalCredit());
+		DetailCategoryResult commonCultureDetailCategoryResult = DetailCategoryResult.create(
+			category.getName(), checkMandatorySatisfaction(takenLectures, category), category.getTotalCredit(),
+			leftCredit, 0);


마찬가지로 DetailCategoryResult를 생성한다음에 isSatisfiedCondition을 바꾸어주는게 더 좋은 흐름이지 않을까 하는 생각!

isSatisfiedCondition은 DetailCategoryResult가 생성될 때 말고는 불변이어야 하는데 pulbic 메서드로 해당 부분을 변경할 수 있게 열어두는건 좋지 않은것 같습니다.

이부분은 전공과목 계산하면서 엣지케이스가 많아서 내가 마지막에 이 값을 바꾸어주려 그랬는데 한번 내가 그럼 기존 방법으로 잘 고민해볼께

stophwan · 2023-08-12T05:10:08Z

src/main/java/com/plzgraduate/myongjigraduatebe/lecture/domain/model/CoreCulture.java

+	public static CoreCulture of(Lecture lecture, CoreCultureCategory category) {
+		return CoreCulture.builder()
+			.lecture(lecture)
+			.coreCultureCategory(category).build();


Suggested change

.coreCultureCategory(category).build();

.coreCultureCategory(category)

.build();

stophwan · 2023-08-12T05:10:56Z

...te/myongjigraduatebe/graduation/domain/service/coreculture/CoreCultureGraduationManager.java

+public class CoreCultureGraduationManager implements GraduationManager<CoreCulture> {
+	@Override
+	public DetailGraduationResult createDetailGraduationResult(StudentInformation studentInformation,


Suggested change

public class CoreCultureGraduationManager implements GraduationManager<CoreCulture> {

@Override

public DetailGraduationResult createDetailGraduationResult(StudentInformation studentInformation,

public class CoreCultureGraduationManager implements GraduationManager<CoreCulture> {

@Override

public DetailGraduationResult createDetailGraduationResult(StudentInformation studentInformation,

stophwan · 2023-08-12T05:36:06Z

...yongjigraduatebe/graduation/domain/service/coreculture/CoreCultureDetailCategoryManager.java

+	private int calculateNormalLeftCredit(Set<Lecture> taken, Set<TakenLecture> finishedTakenLecture,
+		int categoryTotalCredit) {
+		int normalLeftCredit = finishedTakenLecture.stream()
+			.filter(takenLecture -> 문화와예술_예외_과목.contains(takenLecture.getLecture())
+				&& takenLecture.getYear() == 2022
+				&& takenLecture.getSemester() == FIRST)
+			.mapToInt(takenLecture -> takenLecture.getLecture().getCredit())
+			.sum();
+		taken.removeAll(문화와예술_예외_과목);
+
+		int totalTakenCredit = taken.stream()
+			.mapToInt(Lecture::getCredit)
+			.sum();
+		if (totalTakenCredit > categoryTotalCredit) {
+			normalLeftCredit += totalTakenCredit - categoryTotalCredit;
+		}
+		return normalLeftCredit;


여기서 takenCredit을 구한다면 차라리 DetailCategoryResult에서 takenCredit 필드를 final로 만들고 같이 주입시던가 or takenCredit를 getter을 통해 가져오던가 같은 방식이 더 좋은듯!
takenCredit을 구하는 로직이 DetailCategoryResult에도 있고, CoreCultureDetailCategory에도 있으니까. 이 로직도 사실상 모든 클래스에 필요하게 되어버리는것 같아.
DefaultCategoryReuslt도메인의 필드를 final로 한다면 현재 도메인서비스에 필드들을 만들어 도메인으로 만들고 마지막에 매핑을 해주는 방식도 괜찮은것 같아.

사실 이 부분에서 takenCredit을 한번 더 구하는 것은 저 예외 과목들을 제외한 수강과목의 초과 힉점을 계산하기 위해 필요한 단계입니다.
DetailCategory를 생성하기 전 단계의 로직이라 DetailCategoryResult의 takenCredit을 활용 못하는 거 같습니다.
이부분은 예외사항으로 인한 초과 학점(공통교양, 자유선택)으로 넘어가는 학점만 계산하고 takenCredit 초과로 인한 초과 학점은 DetailCategoryResult에서 계산하는 것으로 수정하겠습니다.

5uhwann · 2023-08-12T07:13:15Z

전체적으로 final 변수를 사용함에 있어 오히려 코드 가독성과 응집도 측면에서 오히려 떨어지는 느낌이 들어! 우리 서비스 특성상 DetailCategoryResult의 모든 필드를 final로 한다면 차라리 현재 도메인서비스에 필드를 가지고 매핑하는게 차리리 맞고, 아니면 변경가능한 부분은 메서드를 통해 수정하는게 맞는것같아! 이에 대해 여러가지 코멘트들을 달았는데 수환이 생각이 궁금하네.

일단 final 키워드에 대한 제 생각은 변경 가능성이 없는 것은 처음 생성할 때 확정 짓고 변경 가능성을 강제로 닫아두는 것이 좋다고 생각합니다.

stophwan · 2023-08-12T10:20:25Z

전체적으로 final 변수를 사용함에 있어 오히려 코드 가독성과 응집도 측면에서 오히려 떨어지는 느낌이 들어! 우리 서비스 특성상 DetailCategoryResult의 모든 필드를 final로 한다면 차라리 현재 도메인서비스에 필드를 가지고 매핑하는게 차리리 맞고, 아니면 변경가능한 부분은 메서드를 통해 수정하는게 맞는것같아! 이에 대해 여러가지 코멘트들을 달았는데 수환이 생각이 궁금하네.

일단 final 키워드에 대한 제 생각은 변경 가능성이 없는 것은 처음 생성할 때 확정 짓고 변경 가능성을 강제로 닫아두는 것이 좋다고 생각합니다.

이건 나도 동감하는데 leftCredit같은 필드를 final로 둠으로서 중복코드가 많이 생기는것 같아서 얘기한거야!
근데 생각해보니 이건 final 키워드 때문에 일어나는 문제가 아니고 내가 위의 제사한 방법대로 해결하면 굳이 이런 중복이 안일어날것 같다는 생각이 드는데 어떻게 생각해?

sonarcloud · 2023-08-12T17:17:01Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

stophwan

고생했어! 👍

5uhwann added 14 commits August 11, 2023 18:55

refactor: 누락 로직 추가

d84c6e7

chore: 핵심교양 데이터 추가

402d970

test: 핵심교양 fixture 작성

f213b23

test: 핵심교양 세부 카테고리 결과 테스트 작성

37e952e

feat: 핵심교양 세부 카테고리 결과 반환 구현

8cf4316

test: 불필요 import 제거

592507e

test: fixture 클래스명 변경

65d4f7b

test: 핵심교양 세부 졸업 결과 테스트 작성

3b340f3

test: 핵심교양 세부 졸업 결과 반환 구현

14d529d

refactor: 세부 카테고리 잔여 학점 관련 필드 추가

977d8e4

test: 핵심교양 - 과학과기술 예외사항 로직 테스트 작성

04ad43e

feat: 핵심교양 - 과학과기술 예외사항 로직 구현

c7e9723

test: 핵심교양 문화와예술 예외사항 로직 테스트 작성

297989b

test: 핵심교양 문화와예술 예외사항 로직 구현

af961c1

5uhwann added ✨ feat 새로운 기능 개발 혹은 기존 기능 변경 ♻️ refactor 기본의 로직 변경 없이 코드 개선 labels Aug 11, 2023

5uhwann added this to the 리팩토링 milestone Aug 11, 2023

5uhwann self-assigned this Aug 11, 2023

5uhwann requested a review from stophwan as a code owner August 11, 2023 18:49

stophwan reviewed Aug 12, 2023

View reviewed changes

refactor: DetailCategoryResult leftCredit 계산 로직 수정

100c8ae

stophwan approved these changes Aug 13, 2023

View reviewed changes

5uhwann merged commit 75e9c8f into develop Aug 13, 2023
6 checks passed

5uhwann deleted the feature/핵심교양_졸업_계산 branch August 13, 2023 03:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

핵심교양 졸업 계산 기능 구현 #161

핵심교양 졸업 계산 기능 구현 #161

5uhwann commented Aug 11, 2023

codecov bot commented Aug 11, 2023 •

edited

Loading

stophwan left a comment •

edited

Loading

stophwan Aug 12, 2023

5uhwann Aug 12, 2023

stophwan Aug 12, 2023 •

edited

Loading

stophwan Aug 12, 2023

stophwan Aug 12, 2023

5uhwann Aug 12, 2023

stophwan Aug 12, 2023

stophwan Aug 12, 2023

stophwan Aug 12, 2023

stophwan Aug 12, 2023 •

edited

Loading

5uhwann Aug 12, 2023

5uhwann commented Aug 12, 2023

stophwan commented Aug 12, 2023

sonarcloud bot commented Aug 12, 2023

stophwan left a comment

	.coreCultureCategory(category).build();
	.coreCultureCategory(category)
	.build();

핵심교양 졸업 계산 기능 구현 #161

핵심교양 졸업 계산 기능 구현 #161

Conversation

5uhwann commented Aug 11, 2023

Issue

✅ 작업 내용

codecov bot commented Aug 11, 2023 • edited Loading

Codecov Report

stophwan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stophwan Aug 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stophwan Aug 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

5uhwann commented Aug 12, 2023

stophwan commented Aug 12, 2023

sonarcloud bot commented Aug 12, 2023

stophwan left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 11, 2023 •

edited

Loading

stophwan left a comment •

edited

Loading

stophwan Aug 12, 2023 •

edited

Loading

stophwan Aug 12, 2023 •

edited

Loading