Skip to content

Feature/search #10#30

Open
bnminji wants to merge 6 commits intoprgrms-be-devcourse:developfrom
bnminji:feature/search-#10
Open

Feature/search #10#30
bnminji wants to merge 6 commits intoprgrms-be-devcourse:developfrom
bnminji:feature/search-#10

Conversation

@bnminji
Copy link
Copy Markdown
Collaborator

@bnminji bnminji commented Nov 2, 2021

  • SearchService::findByFilters(+Test) : querydsl 사용하여 구현

  • SearchController (+Test) 구현

  • 1차적인 SearchService구현은 이것으로 마무리합니다.

  • 생각해볼 것 ->
    StartDate, EndDate 검색 방법(예약 달력 api 찾아보기, 뇌피셜 아이디어: mysql 달력)

Copy link
Copy Markdown
Collaborator

@sunH0 sunH0 left a comment

Choose a reason for hiding this comment

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

처음 사용해보는 거라 어려우셨을 텐데 수고하셨어요~
저도 코드 보고 많이 배웠습니다!
현 코드가 동작에 문제가 없다면 우선 merge 하시고 확장하신다 하셨던 부분은 구현해서 다시 pr 날리셔도 될것 같아요!

private String roughAddress;

@Builder
public SearchOneResponse(Room entity) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Respose의 convert해주는 과정에서 생성자보다 static으로 선언한 of 메소드를 해주는 것이 좀 더 명확한 느낌아닌 느낌이 들어요 ㅎㅎ
어떻게 생각하시나요??

Comment on lines +14 to +18
private String location;
private LocalDate startDate;
private LocalDate endDate;
private Guest guest;
private Option option;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

검증 부분이 있으면 좋을 것 같아요~

Comment on lines +14 to +18
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

//@RequestMapping("/v1/search")
@Slf4j
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

없어도 되는 코드일까요??

import org.springframework.data.domain.Pageable;

public interface CustomizedRoomRepository {
Page<Room> findByFiltersOrderByCreatedAt(final SearchRequest searchRequest, Pageable pageable);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

final을 붙인다면 Pageable 도 해주면 좋을 것 같아요!!


public interface RoomRepository extends JpaRepository<Room, Long> {

@Repository
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@repository 가 없으면 오류가 나나요??


//필터별조회
@Transactional(readOnly = true)
public Page<SearchAllResponse> findByFilters(final SearchRequest searchRequest, Pageable pageable){
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

final~

Copy link
Copy Markdown
Collaborator

@BeautterLife BeautterLife left a comment

Choose a reason for hiding this comment

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

query dsl 굿입니다
수고하셨어요

import org.programmers.staybb.domain.room.Room;

@Getter
@Setter
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Setter도 필요한가요?


@Builder
public SearchOneResponse(Room entity) {
this.id = entity.getId();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

id는 필요없지 않을까요?

Comment on lines +24 to +26
this.guest = new Guest(searchRequestModel.getAdult(),
searchRequestModel.getKid(),
searchRequestModel.getTeen());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

builder로 하는건 어떤가요?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Guest 같이 필드가 많이 않은 클래스는 생성자를 사용해도 괜찮지 않을까요??

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.

3 participants