[POM-94] feat: 가게 전화번호 등록 기능 구현 #31
Conversation
juno-junho
requested review from
hyeon-z,
choi5798,
Juhongseok,
beomukim and
ddongpuri
| @PostMapping("/{id}/phone-numbers") | ||
| public void registerPhoneNumber(@RequestBody @Valid PhoneNumberRequest phoneNumberRequest, @PathVariable(value = "id") Long storeId) { |
There was a problem hiding this comment.
전화번호 하나 추가하는 것이면 patchMapping은 어떤가요?
There was a problem hiding this comment.
수정하는것도 있어서 이것은 추후 수정할 생각이에요
There was a problem hiding this comment.
patchmapping은 동일한 메서드로 하나 추가할 생각입니다
src/test/java/com/ray/pominowner/store/domain/PhoneNumberTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: HongSeok Ju <[email protected]>
| import java.util.regex.Pattern; | ||
|
|
||
| @Embeddable | ||
| @EqualsAndHashCode(of = "tel") |
There was a problem hiding this comment.
컬럼이 tel만 존재하는 때 of로 따로 지정해준 이유가 있을까요?
| return requiredStoreInfo.getBusinessNumber(); | ||
| } | ||
|
|
||
| public Store afterRegisterPhoneNumber(String phoneNumber) { |
There was a problem hiding this comment.
registerPhoneNumber앞에 after를 붙여준 이유가 있나요?
There was a problem hiding this comment.
store.afterRegisterPhoneNumber()가 자연스럽다고 생각했어요
메서드 명 정할때 중요한 점이 사용하는 클라이언트 쪽에서 메서드 명을 정해야지 받는쪽 입장에서 메서드 명을 정하는 것은 객체지향적이지 못하다고 생각해요
There was a problem hiding this comment.
반환값이 store 엔티티이기 때문에 after 붙히는게 더 자연스럽다 생각합니다 :)
| .isThrownBy(() -> storeService.registerPhoneNumber(validPhoneNumber, store.getId())); | ||
| } | ||
|
|
||
|
|
| return requiredStoreInfo.getBusinessNumber(); | ||
| } | ||
|
|
||
| public Store afterRegisterPhoneNumber(String phoneNumber) { |
There was a problem hiding this comment.
메소드명만 보고는 폰 번호 등록 이후에 어떤 기능을 수행하는지 잘 모르겠어요
| public String getTel() { | ||
| return tel.getTel(); |
There was a problem hiding this comment.
필드 tel 의 타입과 해당 필드의 getter 타입이 달라요
| public void registerPhoneNumber(String phoneNumber, Long storeId) { | ||
| Store store = findStore(storeId).afterRegisterPhoneNumber(phoneNumber); | ||
| storeRepository.save(store); | ||
| } | ||
|
|
There was a problem hiding this comment.
repostiroy.save() 메소드의 return 값을 뭔가 유용하게 이용할 수 있을 것 같아요
There was a problem hiding this comment.
지금 컨트롤러에 굳이 넘겨 줘야할 필요가 없어서 (자원이 생성된게 아니기 때문에) return 값을 설정하지 않았어요
| @DisplayName("전화번호가 정상적으로 등록된다") | ||
| void successRegisterPhoneNumber() { | ||
| // given | ||
| String validPhoneNumber = "010-1234-5678"; |
There was a problem hiding this comment.
StoreTest 와 이 test 파일간의 유효한 폰 번호 문자열의 포맷이 달라요
There was a problem hiding this comment.
상관없다고 생각해요 어차피 유효한 번호인것만 테스트 하면 되니까요
📌 설명