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

feat: 학점 조회 기능 추가 #366

Merged
merged 7 commits into from
Mar 31, 2024
Merged

feat: 학점 조회 기능 추가 #366

merged 7 commits into from
Mar 31, 2024

Conversation

kwoo28
Copy link
Contributor

@kwoo28 kwoo28 commented Mar 26, 2024

▶ Request

Content

#365

as-is

시간표 조회시 전체학점과 해당학기학점 반환

to-be

TimeTableServiceIml에서 학기조회를 통해 해당 유저의 학점을 계산

✅ Check List

  • 의도치 않은 변경이 일어나지 않았는지.
    • 실수로 라이브러리(pom.xml) 변경이 일어나지 않았는지
    • 병합시 컴파일 & 런타임 에러가 발생하지 않는지
    • 기존 클라이언트와의 호환성 고려가 잘 이루어졌는지
  • 작성한 코드가 프로젝트에 반영됨을 명심하였는지
    • 타인도 알아보고 변경할 수 있는 코드를 작성하였는지
    • 코드 & 커밋 컨벤션을 준수하였는지
    • (필요한) 문서화가 진행되었는지
  • (기능 추가의 경우) 클라이언트의 입장에 대한 충분한 고려가 이루어졌는지
    • 클라이언트 측과 협의가 된 내용인 경우
    • 클라이언트의 요구사항을 잘 반영하는지
    • API 문서에 논리적인 오류 & 가시성이 떨어지는 내용이 없는지

📸 API Document ScreenShot

image

🧪 Test

2개의 학기를 시간표 추가하고 해당학기학점, 전체학기학점를 확인했습니다.

@kwoo28 kwoo28 changed the title Feature: 학점 조회 기능 추가 feat: 학점 조회 기능 추가 Mar 26, 2024
Copy link
Contributor

@daheeParkk daheeParkk left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

@@ -231,5 +238,17 @@ public Map<String, Object> deleteTimeTableById(int id) throws Exception {
}};
}

private int calculateTotalGrades(int userId){
Copy link
Contributor

Choose a reason for hiding this comment

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

A

Suggested change
private int calculateTotalGrades(int userId){
private int calculateTotalGrades(int userId) {

띄어쓰기 해주면 좋을 것 같아요!

Comment on lines 85 to 86
put("이번학기학점", currentGrades);
put("전체학기학점", calculateTotalGrades(user.getId()));
Copy link
Contributor

Choose a reason for hiding this comment

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

C

다른 api처럼 영어로 응답하는게 좋을 것 같아요!

Copy link
Contributor

@daheeParkk daheeParkk left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

Copy link

@duehee duehee left a comment

Choose a reason for hiding this comment

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

고생하셨어요~ 코멘트 확인해주세요!

Map<String, Object> retMap = new HashMap<String, Object>() {{
put("timetable", timetables);
put("semester", semester);
put("grades", currentGrades);
Copy link

@duehee duehee Mar 28, 2024

Choose a reason for hiding this comment

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

C

grade보다는 credit을 사용하는 것은 어떨까요? crade는 학점보다는 성적 느낌이 조금 큰 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

감사합니다. 근데 DB에는 이수학점이 grade로 돼있는데.. 관습상 grade로 통일하는건 어떤가요..?

Copy link

Choose a reason for hiding this comment

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

앗..! 그렇다면 그냥 grade를 사용하는 게 좋아보이네요!

Map<String, Object> retMap = new HashMap<String, Object>() {{
put("timetable", timetables);
put("semester", semester);
put("grades", currentGrades);
put("total_grades", calculateTotalGrades(user.getId()));
Copy link

Choose a reason for hiding this comment

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

C

위와 동일합니다

Copy link

@duehee duehee left a comment

Choose a reason for hiding this comment

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

고생하셨어요~ 리뷰 확인해주세요!
-> ###을 사용했는데도 글씨가 안 커져요 ㅜㅜㅜ

@kwoo28 kwoo28 merged commit bc7d9b7 into develop Mar 31, 2024
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.

4 participants