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

프로젝트의 전반적인 리뷰 #3

Open
Tianea2160 opened this issue Jan 30, 2024 · 1 comment
Open

프로젝트의 전반적인 리뷰 #3

Tianea2160 opened this issue Jan 30, 2024 · 1 comment

Comments

@Tianea2160
Copy link

안녕하세요.
해당 프로젝트의 코드를 쭉 읽어보고서 드는 생각들을 정리해보려고 합니다.

장점

  • 멀티모듈을 사용하여서 역할을 분리하고 레이어 분리를 잘하시려고 노력하신 부분이 눈에 띄게 보입니다.
  • 코틀린에 대해나 이해도가 높으셔서 계층을 나누시려고 하신 부분은 흥미롭게 보았습니다.
  • 실제로 여러 오픈소스에서 볼법한 공통 용어나 구조를 볼 수 있어서 재미있었습니다.

개선할 점

  • 사용자의 관점에서 즉 도메인 로직을 사용하려는 제 3자의 입장에서 보았을 때 이걸 어떻게 써야할지 잘 모르겠습니다.

정확히는 여기에 무슨 클래스가 있고 어떤 commmand를 사용해야하는지 모두 알고 있어야지 사용할 수 있는 모듈이라고 생각합니다.
예를들어서 제가 파일을 저장하려고 합니다. 저는 S3에 파일 하나를 업로드하려고 하면 사용자는 s3라는 정보와 파일 하나를 가지고 있고 다른 정보는 아무것도 없을 것입니다. 그럼에도 직관적으로 upload, create 메소드만 호출하여서 파일을 업로드 할 수 있어야한다고 생각합니다. 왜냐하면 복잡성은 다양한 기능을 제공해줄 수 있지만 이로 인해서 허들이 높아져서 아무도 안쓰는 나만의 모듈이 될 수 도 있기 때문입니다.

ex)

XXXClient.upload(S3, file)
XXXClient.upload(LOCAL, file)

  • 깊이가 너무 깊습니다

제가 domain 로직을 하나 이해하기 위해서 presentation 로직까지 갔다가 와야합니다(예를 들어서 confiuration 정보가 로직 시작부터 끝날때까지 parameter로 전달됩니다). 단순히 upload하나 하는데 이렇게 까지 해야하는 건지 저는 잘 모르겠습니다. 만약 해당 프로젝트의 목적이 사용자가 편하게 사용하는 기능을 개발하자가 목적이 아니라 내가 멀티모듈을 한번 써보고 싶고 코틀린을 그냥 잘 써보고 싶다고 생각해서 하신거라면 지금도 괜찮다고 생각합니다.
차라리 제니릭 및 Mark interface 형식을 사용해보시는 것도 추천드립니다
ex) XXXCommand -> Command 인터페이스 만들어서 상속, XXXAction -> 액션 인터페이스 만들어서 상속

  • executeAction과 같은 메소드는 없어도 될 것 같습니다.

해당 로직이 비동기 이거나 별도의 thread에서 동작하는 것이라면 괜찮은데 이건 동기로직이라서 크게 의미 없는 것 같습니다
processorFactory.create(storeAction)
.executeAction()

여러 내용을 적었지만 저도 단편적으로 보고 잘못 이해한 부분이 있을 수 있고 작성자의 의도를 제대로 파악하지 못했을 수 있을 것 같습니다.

@JY-Dev
Copy link
Owner

JY-Dev commented Feb 2, 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

No branches or pull requests

2 participants