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

studio 조회 관련 기능 개발 #28

Merged
merged 25 commits into from
Jan 15, 2024
Merged

Conversation

oownahcohc
Copy link

@oownahcohc oownahcohc commented Dec 2, 2023

관련 이슈

  • studio 조회 관련 기능 개발 #25
  • entity postfix 추가 #22 : 해당 이슈 관련해서 질문 드릴게 있어서 언급했습니다. 저는 도메인을 pure 하게 가져가기 위해서는 도메인 엔티티가 영속성 엔티티를 몰라야 한다고 생각해서, StudioEntity 내부에 toStudio 메서드를 두어 매핑해주는 형태로 가져갔는데요, 어떻게 생각하시나요?!

studio 전체 조회

현재 studio 데이터가 적기 때문에, 전체 조회 시 모든 스튜디오를 내려주는 것으로 개발 진행했습니다.

studio 상세 조회

  • 기존 studio 의 time, price 컬럼 타입이 TEXT 였기 때문에, 해당 부분 api response 가 매우 자유분방합니다. api response 관련해서 클라이언트와 논의가 필요할 듯 합니다. [ 현상소 데이터 ]
  • 최대한 맞추고자 RunnigTimeEntityOperationStatus, DayOfWeekEVERY_DAY, WEEK_DAY, WEEK_END 등 추가했습니다.
  • time, price 의 response 형식은 도메인 로직과는 별개로 사용자 인터페이스와 데이터 표현 관련 로직(뷰 로직)으로 판단해서, 각각 뷰 모델로 분리했습니다.

studio 검색

studio 검색 관련해서는 글 작성한게 살짝 있는데 다듬어지는대로 #27 이슈에 공유하도록 할게요!!

  • InputKeyword : 사용자가 입력한 검색어에 대한 객체입니다. 공백 기준으로 입력 검색어를 split 합니다
  • KeywordManager : 공백 기준으로 나뉜 inputKeyword 의 토근에 대해서 각각 등급을 설정하는데요, 저희 서비스는 어차피 현상소, 스튜디오 검색이기 때문에 ~ 현상소, ~ 스튜디오 식으로 들어오는 검색어에서 현상소, 스튜디오 는 검색어 키워드로서의 가치가 낮다고 판단했습니다. 따라서 이와 관련된 단어들을 StopWord 로 설정해놓고 이외의 token들의 가치를 더 높게 설정해서 비즈니스 검색어를 만들어주도록 짰습니다.

@oownahcohc oownahcohc self-assigned this Dec 2, 2023
Copy link
Contributor

@daehwan2da daehwan2da left a comment

Choose a reason for hiding this comment

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

  1. 레이어간 dto naming 이 모호한데요, 간단히 컨벤션 정의해 프로젝트 전반에 적용되면 좋을것같습니다
    (Api) <- ~Result ( View or Result 의견 주세요 ~)_ -> (Service) <- domain model -> (Service Component)

  2. 전반적으로 레이어 dto 간 의존이 혼동되고있는데요, top-down 으로 통일되면 좋을것같습니다

  • Api package 의 class 들은 Service Layer 에서 import 되지 않도록
    • ServiceLayer 에서 Response 를만들어 Api 로 전달하는것이 아닌, 전달될 dto 를 만들어 응답, Api 에서 dto 를 받아서 Response 구성
  1. base/20231126 부시 새롭게 해두겠습니다. 리베이스 한번 하시고 푸시 부탁드릴게요

this.values = values;
}

public static InputKeyword from(String keyword) {
Copy link
Contributor

Choose a reason for hiding this comment

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

keyword 가 null 로 들어올 경우, emptyList 로 채워주어야할것같습니다. (NPE 방어로직 구현 필요)

this.modified = modified;
}

static List<String> replaceStopWordToModifiedWord(List<String> keywordTokens) {
Copy link
Contributor

Choose a reason for hiding this comment

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

unit test 작성 부탁드리겠습니다~

.toList();
}

static void removeStopWordIn(List<String> keywordTokens) {
Copy link
Contributor

Choose a reason for hiding this comment

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

unit test 작성 부탁드리겠습니다~


import org.jetbrains.annotations.NotNull;

class KeywordManager {
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 클래스에 대해서 test case 정리해서 test 작성되면 좋을것같습니다


import com.teamfillin.fillin.domain.runningTime.RunningTime;

abstract class RunningTimeInfoFormatter {
Copy link
Contributor

Choose a reason for hiding this comment

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

구현체들에 대해서 unit test 있으면 좋을것같습니다

@daehwan2da
Copy link
Contributor

https://github.com/TeamFILL-IN/server-renew/pull/33/files
해당 PR 참고해 api response 를 ResponseEntity 로 변경 부탁드립니다!
(status code 를 담기 위해 변경했었습니다.)

@oownahcohc
Copy link
Author

https://github.com/TeamFILL-IN/server-renew/pull/33/files
해당 PR 참고해 api response 를 ResponseEntity 로 변경 부탁드립니다!

넵 확인했습니다!!

@oownahcohc oownahcohc merged commit 1642e1b into base/20231126 Jan 15, 2024
@oownahcohc oownahcohc deleted the feature/issue/25 branch January 15, 2024 17:31
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.

2 participants