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

refactor: 시간표 등록시 학점 정보 추가 완료 #19

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

SongJaeHoonn
Copy link

Summary

#18

시간표 등록시 추가한 강의들의 전체 학점 정보를 보여주기 위해 LectureSelectResponseDtocredit 정보를 추가했습니다.

Tasks

  • LectureSelectResponseDtocredit 필드 추가

ETC

Screenshot

image

@SongJaeHoonn SongJaeHoonn added the 🔨 Refactor 코드 수정 및 개선 label Sep 4, 2024
@SongJaeHoonn SongJaeHoonn self-assigned this Sep 4, 2024
@SongJaeHoonn SongJaeHoonn linked an issue Sep 4, 2024 that may be closed by this pull request
1 task
@@ -17,6 +17,7 @@ public class LectureSelectResponseDto {
private String professor;
private String room;
private String time;
private Integer credit;
Copy link
Contributor

Choose a reason for hiding this comment

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

credit이라는 단어는 맥락에 따라 다양한 의미를 가질 수 있기 때문에, 코드에서 사용될 때 직관적으로 이해하기 어려울 것 같아요. 학교 시스템 같은 환경에서는 이러한 단어를 한글 발음 그대로 영어로 옮기는 방식(hakjeom 등)이 종종 사용되는 것을 볼 수 있어요. 외에 토스 라이브러리에서도 이러한 방식이 사용된 것을 보았고, 해당 코드에서도 사용자의 이해를 돕기 위해 변수명을 hakjeom처럼 발음 그대로 사용하는건 어떨까 싶어요.

토스 라이브러리 한글 발음 변수명 사용 사례

https://github.com/toss/es-hangul/blob/main/src/_internal/hangul.ts

  if (
    hasBatchim(source, {
      only: 'single',
    }) &&
    canBeJongseong(`${lastConsonant}${nextCharacter}`)
  ) {
    return combineJongseong(`${lastConsonant}${nextCharacter}`);
  }

이거는 제 의견일 뿐 이렇게 해보는건 어떤지에 대한 제안입니다.
해당 내용에 대해 @mingmingmon 님의 의견도 궁금해요.

Copy link
Author

@SongJaeHoonn SongJaeHoonn Sep 4, 2024

Choose a reason for hiding this comment

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

DB의 credithakjeom으로 바꾸게 된다면 다른 헷갈리는 필드(type, category, groupName 등)도 변경하는 것이 좋을 것 같아요.

하지만 프론트엔드에서도 코드를 수정해야 한다는 불편함이 있고, 과목 정보를 스크래핑 할때도 코드를 수정해야 해서, 저도 @mingmingmon 님과, @SWARVY 님의 의견을 들어보는 것이 좋을 것 같습니다.

Copy link

@SWARVY SWARVY Sep 4, 2024

Choose a reason for hiding this comment

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

해당 문제의 경우 프론트엔드에서도 동일하게 발생합니다. 백엔드에서 제공하는 명세서를 바탕으로 타입설계를 하기 때문입니다

하여, 표현이 모호한 변수명들을 바꾸는 것은 프론트엔드의 코스트가 발생하지만 유지보수적 관점으로 보았을 때 명확한 표현으로 교체하는것이 장기적으로 더 좋아보입니다.

바꾸게되면 변경사항 알려주세요 👍🏻

Copy link
Collaborator

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
🔨 Refactor 코드 수정 및 개선
Projects
None yet
Development

Successfully merging this pull request may close these issues.

시간표 등록시 학점 정보 추가
4 participants