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/answer service and test #144

Merged
merged 13 commits into from
Nov 6, 2023
Merged

Conversation

dasd412
Copy link
Contributor

@dasd412 dasd412 commented Nov 3, 2023

리팩토링 : answer_service.py 와 테스트 코드 다수

  1. 더 이상 사용되지 않는 create_chat_openai() 함수를 llm_factory에서 제거
  2. answer_service.py 응집성 향상을 위한 리팩토링 (Mixin 도입, 클래스 추출, Strategy 패턴 도입)
  3. 테스트 코드에서 @patch() 파라미터가 문자열 하드 코딩인 것들 모두 Mixin의 메서드를 활용해서 대체
  4. 주석 제거 및 docstring 도입
  5. import 문 순서 정리 (라이브러리 -> config -> service -> repository->domain->modules->utils 순)

참고 사항

python Mixin이란?

일반적으로, 믹스인은 단순하고, 잘 정의된 행동을 가지며, 가능한 한 다른 클래스와의 결합도를 낮추어야 합니다.

장점

다중 상속의 간편성: 믹스인을 통해 다중 상속의 이점을 활용할 수 있습니다. 이를 통해 여러 클래스 간에 코드를 공유하며, 단일 상속에서는 불가능한 유연성을 얻을 수 있습니다.

모듈성: 믹스인은 특정 기능이나 행위를 모듈화하여 다른 클래스에 쉽게 추가할 수 있게 합니다. 이로 인해 각 기능을 독립적으로 유지하고 테스트할 수 있습니다.

재사용성 향상: 공통적으로 사용되는 기능을 믹스인으로 분리함으로써, 여러 클래스에서 같은 코드를 재사용할 수 있어 DRY(Don't Repeat Yourself) 원칙을 따를 수 있습니다.

확장성: 새로운 기능을 추가하거나 기존 기능을 변경할 때, 관련 믹스인을 수정하거나 새로운 믹스인을 추가하기만 하면 됩니다. 이는 전체 시스템에 영향을 미치는 대규모 변경을 피할 수 있게 해줍니다.

명확한 의존성: 클래스가 필요로 하는 기능이 믹스인으로 명시적으로 표현됩니다. 이는 코드의 가독성과 이해도를 높이며, 개발자가 어떤 기능이 구현되어 있는지 쉽게 파악할 수 있게 합니다.

동적 기능 추가: 런타임에 믹스인을 통해 객체에 동적으로 기능을 추가할 수 있습니다. 이는 특정 상황에서 유용하게 활용될 수 있습니다.

유지보수의 용이성: 믹스인으로 코드가 잘 분리되어 있으면, 한 곳에서의 변경이 다른 부분에 미치는 영향을 최소화할 수 있어 유지보수가 용이해집니다.

클래스와 메서드의 단순화: 단일 책임 원칙을 따르게 하여, 각 클래스와 메서드가 더 작고 관리하기 쉬운 단위로 유지될 수 있도록 합니다.

믹스인을 올바르게 사용하면 이러한 장점들을 극대화하여 프로그램의 전반적인 설계를 향상시킬 수 있습니다.

단점

네임스페이스 오염:
믹스인은 다중 상속의 한 형태를 제공하기 때문에, 많은 메서드들이 클래스에 추가될 수 있습니다. 이로 인해 클래스의 네임스페이스가 오염될 수 있으며, 예상치 못한 이름 충돌이 발생할 수 있습니다.

상속 체인의 복잡성 증가:
여러 믹스인이 함께 사용될 때, 상속 체인이 복잡해지고, 이로 인해 코드를 이해하고 디버그하기가 어려워질 수 있습니다. 상속 관계가 깊어질수록 클래스의 동작을 추적하기가 더 어려워집니다.

메서드 충돌:
둘 이상의 믹스인이 동일한 메서드를 정의하고 있을 때, 상속 순서에 따라 어느 메서드가 호출될지 예측하기 어려울 수 있습니다. 이는 다중 상속에서 일반적으로 발생할 수 있는 문제입니다.

의존성 문제:
믹스인은 때때로 다른 클래스나 모듈에 대한 은밀한 의존성을 가질 수 있습니다. 이는 코드의 결합도를 높이고 유지보수를 어렵게 만듭니다.

오용의 가능성:
믹스인은 그 목적이 명확히 정의되어 있어야 합니다. 그렇지 않으면 개발자들이 믹스인을 오용할 수 있으며, 이는 의도하지 않은 방식으로 코드 베이스에 영향을 줄 수 있습니다.

테스트의 어려움:
믹스인은 다른 클래스와 결합하여 사용될 때만 의미가 있기 때문에, 독립적으로 테스트하기가 어렵습니다. 또한 믹스인을 사용하는 각 클래스에 대해 추가적인 테스트가 필요할 수 있습니다.

문서화의 어려움:
믹스인을 사용하면 클래스의 동작을 문서화하기가 더 복잡해질 수 있습니다. 사용자는 믹스인에 정의된 동작뿐만 아니라, 그것이 상속받는 모든 클래스에서의 동작도 이해해야 할 수 있습니다.

믹스인을 사용할 때는 이러한 문제점들을 염두에 두고 신중하게 설계하는 것이 중요합니다. 

…를 따로 만듬 (SRP,OCP)

2. 테스트 코드 내에서 변경이 될 때마다 깨지기 쉬운 Class path를 관리하기 위해 mixin 도입
@dasd412 dasd412 added refactor 코드 리팩토링 test 테스트 코드, 리팩터링 테스트 코드 추가(프로덕션 코드 변경 X) rename 파일 혹은 폴더명을 수정하거나 옮기는 작업만인 경우 remove 파일을 삭제하는 작업만 수행한 경우 labels Nov 3, 2023
@dasd412 dasd412 self-assigned this Nov 3, 2023
@dasd412
Copy link
Contributor Author

dasd412 commented Nov 3, 2023

SRP (Single Responsibility Principle)

정의: 한 클래스는 하나의 책임만 가져야 한다는 원칙입니다.
목적: 이는 클래스를 변경해야 하는 이유가 단 하나여야 함을 의미합니다.
이점: 코드의 재사용성과 유지보수성이 향상됩니다. 또한 코드를 이해하기 쉽고, 단위 테스트를 수행하기 더 쉬워집니다.

OCP (Open-Closed Principle)

정의: 소프트웨어의 엔티티(클래스, 모듈, 함수 등)는 확장에는 열려 있어야 하지만 변경에는 닫혀 있어야 한다는 원칙입니다.
목적: 기존의 코드를 변경하지 않고도 시스템의 기능을 변경하거나 확장할 수 있어야 합니다.
이점: 시스템을 쉽게 확장할 수 있으며, 기존 코드의 안정성과 유지보수성이 보장됩니다.

Copy link
Contributor

@saebyeok0306 saebyeok0306 left a comment

Choose a reason for hiding this comment

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

리팩토링 깔끔하네요.



class LightQuestionGiver(metaclass=SingletonMeta):
class LightQuestionGiver(DirectoryMixin, metaclass=SingletonMeta):

Copy link
Contributor

Choose a reason for hiding this comment

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

Mixin 패턴을 알고는 있었지만 실제로 활용하는 건 처음 봤네요.

Comment on lines +1 to +6
class DirectoryMixin:
"""
Python에서 믹스인(Mixin)은 특정 클래스에 메서드를 제공하기 위해 다른 클래스에 "믹스인"될 수 있는 클래스를 말합니다. 믹스인은 다른 클래스의 기능을 확장하거나 수정할 목적으로 사용되며, 보통 단독으로는 인스턴스화되지 않습니다. 이는 다중 상속의 한 형태로서, 여러 클래스에서 필요로 하는 공통 기능을 제공하는 재사용 가능한 클래스입니다.

믹스인의 주요 목적은 코드 중복을 줄이고, 클래스 간에 기능을 공유하여 각 클래스를 더 작고, 관리하기 쉽고, 단일 책임 원칙을 따르도록 만드는 것입니다.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

결국 상속해서 쓰는 개념이네요. 조심해서 써야겠군요.

@saebyeok0306 saebyeok0306 merged commit 2612b86 into dev Nov 6, 2023
2 checks passed
@saebyeok0306 saebyeok0306 deleted the refactor/answer_service_and_test branch November 6, 2023 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor 코드 리팩토링 remove 파일을 삭제하는 작업만 수행한 경우 rename 파일 혹은 폴더명을 수정하거나 옮기는 작업만인 경우 test 테스트 코드, 리팩터링 테스트 코드 추가(프로덕션 코드 변경 X)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants