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: 추천 검색어 자동완성 API 구현 #208

Merged
merged 6 commits into from
Jul 3, 2024
Merged

feat: 추천 검색어 자동완성 API 구현 #208

merged 6 commits into from
Jul 3, 2024

Conversation

PgmJun
Copy link
Member

@PgmJun PgmJun commented Jun 25, 2024

issue: #207

as-is

식당 검색 시에 자동완성 기능이 없어서 실제로 검색해보지 않으면 검색어와 관련된 이름의 식당이 어떤 것이 있는 지 알 수 있었습니다.
이로인해 사용자들이 불편을 겪고 있음을 파악하여 이를 수정해보고자 하였습니다.

to-be

  • 식당 검색 시, 사용자들의 사용성을 높이기 위해 자동완성 기능을 구현하였습니다.

    • 자동완성을 통해 조회할 수 있는 식당의 개수는 최대 5개입니다.
  • 자동완성으로 조회되는 데이터는 좋아요 기준 내림차순으로 정렬합니다.

처음엔 정렬을 할 지 말 지 고민이 있었는데요.
고민의 이유는 '검색어 자동완성' 이라는 맥락 때문이었습니자.
"자동완성은 그냥 검색어에 일치하는 데이터를 보여주는 거지 추천까지 해도 되는걸까?" 라는 생각이었습니다.
네이버 같은 보통의 검색 서비스도 최신순/정확도순 으로 자동완성을 만드는 것 같았어요.

하지만 이러한 최신순/정확도순이라는 기준은 평가하기 어려운 다양한 주제를 모두 다루는 경우 사용될 수 있는 기준이라고 생각합니다.
저희 서비스는 맛집 추천이라는 기준을 가지고 있기 때문에 자동완성의 기준을 더 다양하게 잡아볼 수 있을 것 같아요.
그래서 결론적으로 좋아요 수를 기준으로 자동완성을 처리해보도록 하겠습니다.
추가로 맥락에 맞게 '검색어 자동완성'보다는 '검색어 추천'이라고 생각하고 코드를 작성해보았습니다.

의견

조회 size 조절은 서버에서

@GetMapping("/campuses/{campusId}/restaurants/search/autocomplete")
    public ResponseEntity<RestaurantSearchesResponse> autocompleteSearchRestaurants(@PathVariable final Long campusId,
                                                                                        @RequestParam final String namePrefix) {
        return ResponseEntity.ok(restaurantService.findByRestaurantNamePrefix(campusId, namePrefix, Pageable.ofSize(5)));
    }
public RestaurantSearchesResponse findByRestaurantNamePrefix(final Long campusId, final String namePrefix, final Pageable pageable) {
        List<Restaurant> restaurants = restaurantRepository.findByNamePrefix(campusId, namePrefix, pageable);

        return RestaurantSearchesResponse.from(restaurants);
    }

조회 size 제한 설정을 Request Parameter를 통해 클라이언트 측에 맡기면,
URL을 통해 원하는 size만큼 요청이 가능하기 때문에 악의적인 요청이 반복적으로 발생하는 경우 메모리 관련 문제로 이어질 수 있다는 생각에
size를 서버에서 조절하도록 로직을 작성하였습니다.
이 부분에 대한 의견도 함께 주시면 감사하겠습니다.

@PgmJun PgmJun self-assigned this Jun 25, 2024
@PgmJun PgmJun added the feature label Jun 25, 2024
Copy link
Contributor

@hyeon0208 hyeon0208 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 이든 ~ 😀
간단한 부분들과 실수하신 부분 리뷰남기고 가니 확인해주시면 감사하겠습니다 😁
추가로 질문에 대한 의견을 아래에 남겨 놓고 갑니다 😄

조회 size 조절은 서버에서

size를 조절 가능한 위치를 정하는 건 정책에 다를 것 같다고 생각합니다
만약 사용자가 자동완성 데이터 수를 원하는 만큼 볼 수 있도록 한다면
서버에서는 이든이 말씀하신 문제를 방지하기 위해 size 제한을 걸어 둘 필요가 있어보이고
아니라면 지금 이든이 구현하신 대로 서버에서 정해진 수 만큼으로 조절해주는 것이 좋다고 생각해요😀

저는 개인적으로 자동완성으로 보여질 데이터 수를 사용자가 임의로 정할 필요는 없다고 생각해 지금이 좋은 것 같아요 😊

자동완성 조회 기능에 정렬이 필요할까요?

단순한 가나다 순의 정렬이라면 저 또한 정렬은 필요하지 않다고 생각해요
하지만 별점 순 또는 좋아요 수(찜 수)로 정렬되는 것은 나쁘지 않다고 생각합니다
보통 사용자가 검색을 할 때는 자신이 알아낸 인기 맛집을 가기 위해서
검색해 찾아볼 것이라고 생각하는데
위 순으로 정렬된다면 상위부터 차례로 보는 사람 특성상 자신의 원하는 맛집을
빠르게 확인할 수 있어 사용자 경험이 좀 더 좋아 질 것 같다는 의견입니다 😄

this.name = name;
}

public static RestaurantSearchResponse from(Restaurant restaurant) {
Copy link
Contributor

Choose a reason for hiding this comment

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

RestaurantServicefindByRestaurantNamePrefix메서드 파라미터에는 final 키워드를 붙여주셨더라구요
지금 프로젝트 코드 전체적으로 final 키워드가 많이 사용되어 있지만 누락되어 있는 부분도 많은데
이든은 final 키워드를 명시해준다 안한다 어떤 파 이신가요 ? 😀

저는 반반이지만 지금 처럼 전체적으로 final 키워드가 많이 사용되어 있다면
이에 맞춰서 저희가 추가하는 코드 또는 수정하는 코드에 final 키워드를 명시해주는 것으로
컨벤션을 잡으면 어떨 지 의견 드립니다 ! 🤗

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 final 키워드가 사용되는 곳도 있고 안된 곳도 있어서 어떻게할지 고민하다가
우선은 final 처리가 되어있는 쪽은 하고 안되어있는 쪽은 안하는 식으로 개발했는데요!

여쭤보신 부분에 대해 답변드리자면
저는 final을 선호하는 편입니다.
딱히 재할당이 필요하지 않다면 입력받은 매개변수의 재할당을 막아두어야 나중에 어디서 값이 변경되었는지 몰라 트러블슈팅에 난항을 겪는 일도 없을 것 같네요 😄
난항을 겪지 않더라도 그럴 가능성 자체를 차단항 수 있다는 점이 장점인 것 같아요~!

private Long id;
private String name;

public RestaurantSearchResponse() {
Copy link
Contributor

Choose a reason for hiding this comment

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

기본 생성자로 인스턴스를 생성할 것이 아니니 private 여도 될 것 같습니다 !

Copy link
Member Author

@PgmJun PgmJun Jun 25, 2024

Choose a reason for hiding this comment

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

확인 감사합니다~~

public class RestaurantSearchesResponse {
List<RestaurantSearchResponse> restaurants;

public RestaurantSearchesResponse() {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto 🎈

Copy link
Member Author

Choose a reason for hiding this comment

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

뉴진스이신가요??


@Getter
public class RestaurantSearchesResponse {
List<RestaurantSearchResponse> restaurants;
Copy link
Contributor

Choose a reason for hiding this comment

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

실수로 누락하신 것 같습니다!😀

Suggested change
List<RestaurantSearchResponse> restaurants;
private List<RestaurantSearchResponse> restaurants;

Copy link
Member Author

@PgmJun PgmJun Jun 25, 2024

Choose a reason for hiding this comment

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

오 그렇네요 확인 감사합니다!

@PgmJun
Copy link
Member Author

PgmJun commented Jun 25, 2024

@hyeon0208

하지만 별점 순 또는 좋아요 수(찜 수)로 정렬되는 것

고민하던 부분인데 저랑 생각이 비슷하네요!
구현을 바로 하지 않고 고민한 이유는 '검색어 자동완성' 이라는 맥락 때문이었는데요
"자동완성은 그냥 검색어에 일치하는 데이터를 보여주는 거지 추천까지 해도 되는걸까?" 라는 생각이었습니다.
네이버 같은 보통의 검색 서비스도 최신순/정확도순 으로 자동완성을 만드는 것 같았어요.

하지만 이러한 최신순/정확도순이라는 기준은 평가하기 어려운 다양한 주제를 모두 다루기 때문에 있는 기준이라고 생각합니다.
저희 서비스는 더 나은 맛집 추천이라는 기준을 가지고 있기 때문에 자동완성의 기준을 더 다양하게 잡아볼 수 있을 것 같아요.
그래서 결론적으로 좋아요 수를 기준으로 자동완성을 처리해보도록 하겠습니다.
추가로 맥락에 맞게 '검색어 자동완성'보다는 '검색어 추천'이라고 생각하고 코드를 조금 변경해보겠습니다.

@PgmJun PgmJun changed the title feat: 캠퍼스 식당 이름 자동완성 검색 API 구현 feat: 캠퍼스 추천 식당 자동완성 검색 API 구현 Jun 25, 2024
@PgmJun PgmJun changed the title feat: 캠퍼스 추천 식당 자동완성 검색 API 구현 feat: 캠퍼스 추천 검색어 자동완성 API 구현 Jun 25, 2024
@PgmJun PgmJun changed the title feat: 캠퍼스 추천 검색어 자동완성 API 구현 feat: 추천 검색어 자동완성 API 구현 Jun 25, 2024
Copy link
Contributor

@Arachneee Arachneee left a comment

Choose a reason for hiding this comment

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

코드가 전체적으로 깔끔해서 보기 좋네요 👍

Comment on lines +150 to +152
() -> assertThat(response.as(RestaurantSearchesResponse.class).getRestaurants()).extracting("name")
.containsExactly(names),
() -> assertThat(response.as(RestaurantSearchesResponse.class).getRestaurants()).hasSize(size)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
() -> assertThat(response.as(RestaurantSearchesResponse.class).getRestaurants()).extracting("name")
.containsExactly(names),
() -> assertThat(response.as(RestaurantSearchesResponse.class).getRestaurants()).hasSize(size)
() -> assertThat(response.as(RestaurantSearchesResponse.class).getRestaurants()).hasSize(size)
.extracting("name")
.containsExactly(names)

이렇게 좀 더 간결하게 표현해도 좋을 것 같아요!

Copy link
Member Author

@PgmJun PgmJun Jun 30, 2024

Choose a reason for hiding this comment

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

좋은 의견 감사합니다:)
하지만 저는 그렇게 작성해도 좋지만 '사이즈가 몇 인가' 와 'names를 포함하고 있는가' 는 다른 테스트라고 생각해서 분리해두었습니다.

테스트라는 것은 물론 코드 검증 역할도 있지만, 문서의 역할도 한다고 생각해서
성능 상 문제가 발생하는 부분이 아니라면 알아보기 쉬운 코드를 선택해서 작성하는 편인데 바꾸는 것이 좋을까요??

Copy link
Contributor

Choose a reason for hiding this comment

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

캠퍼스의_식당_중_입력받은_접두사로_시작하는_이름의_식당을_좋아요_기준_내림차순으로_조회한다 테스트 코드의 방식과 통일성이 안맞아보여서 남겼습니다.

테스트하고자 하는 부분이 정확한 응답 바디를 주고 있는지 확인하는 것이기 때문에 한줄로 줄여도 알아보는데 어려움은 없을 것 같아요.

Copy link
Member Author

Choose a reason for hiding this comment

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

말씀해주셔서 다시 봤는데 그 부분은 제가 3개가 잘 오는지 개인적으로 검증하려고 코드를 써두고 지우지 못했네요!
추가로 containsAnyOf 로 값을 검증하고 있었는데 3개가 순서에 맞게 제대로 조회되어왔는 지 검사하는 것이 목적이기 때문에 containsExactly로 변경하였습니다:)

Copy link

sonarcloud bot commented Jul 3, 2024

@PgmJun PgmJun merged commit 25221b4 into main Jul 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants