-
Notifications
You must be signed in to change notification settings - Fork 179
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
[Spring 장바구니 - 2단계] 조동현(후디) 미션 제출합니다. #130
Changes from 13 commits
6de48aa
1069430
44f8494
52f4fae
dc98d78
a4ce601
8124935
85a0ec9
b8f6767
a2a0846
aba2859
9ec5601
393d7d9
a7e5756
3333130
0e807fd
62b4f6f
7fa5bfb
ba4b053
a0b80fe
5a6fd00
9fb21e8
7d74ce3
6842fb2
c6e0af3
3d22fc9
b957caf
4e87602
270d77d
202aff9
8b8e89f
c07cca7
a6d7807
54fab62
f72463b
4f54ff4
b835c77
4e4cd9b
adef1f1
22a3c36
9ccb697
0c4ab1b
f7a2d0c
d5199a3
60e106a
38f6e7f
d79ef38
f7e3a5b
7878409
3d4b09d
b75374c
a1e45dc
956ee2f
946516f
bc35f65
19c820a
58bcd51
81b1c5d
629133f
0f6eaaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,14 @@ | ||
package woowacourse.shoppingcart.dao; | ||
|
||
import java.util.Optional; | ||
import woowacourse.shoppingcart.entity.AddressEntity; | ||
|
||
public interface AddressDao { | ||
void save(AddressEntity addressEntity); | ||
void save(long customerId, AddressEntity addressEntity); | ||
|
||
AddressEntity findById(int id); | ||
Optional<AddressEntity> findById(long customerId); | ||
|
||
void update(AddressEntity addressEntity); | ||
void update(long customerId, AddressEntity addressEntity); | ||
|
||
void delete(int customerId); | ||
void delete(long customerId); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
package woowacourse.shoppingcart.dao; | ||
|
||
import java.util.List; | ||
import java.util.Optional; | ||
import woowacourse.shoppingcart.domain.CartItem; | ||
import woowacourse.shoppingcart.entity.CartItemEntity; | ||
|
||
public interface CartItemDao { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dao를 인터페이스로 분리한 이유가 있을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 나중에는 jdbc를 사용하지 않고 데이터베이스에 접근하는 구현체가 있을수도 있겠다는 생각에 만들어두었는데, 오버엔지니어링 같기도하고, DAO 계층에만 인터페이스를 만들어둔것이 일관성이 없어 보이기도 합니다!.. 스티치는 어떻게 생각하시나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저같은 경우에는 예상되어지는 부분에 대한 추상화는 진행하지만 그렇지 않은 부분에 대해서는 최대한 오버엔지니어링을 피하려고 하고 있어요!! 후디가 작성한 코드에서 추후 다른 DB 리소스를 붙일 상황이 예상된다거나 그러한 요구사항이 추후 존재한다는 걸 알고 있다면 지금의 추상화는 좋다고 생각해요!! 하지만 '혹시 모르니깐'의 개발은 오버엔지니어링의 시작점이라고 생각합니다!! 지금 다양한 방법으로 시도해보고 수정해보는 것은 좋다고 생각해요!! 제가 리뷰를 남긴 이유도 정말 어떠한 이유가 존재해서 인터페이스로 분리한 것인지 궁금해서 여쭤본 것이었습니다 :) |
||
CartItemEntity findById(Long cartItemId); | ||
Optional<CartItemEntity> findById(Long cartItemId); | ||
|
||
List<CartItemEntity> findAllByCustomerId(Long customerId); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 url도 함께 고민해보면 좋을 것 같아요!! path parameter를 쓰는게 적절한지 한번 고민해보면 좋을 것 같습니다 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀해주신 API는 특정 유저의 장바구니에
productId
에 해당하는 제품을 담고 있는지 알려주는 API 입니다! 저는 Path Parameter가 제외된 구현을 떠올리기 어려운데, 어떻게 하면 Path Parameter 를 사용하지 않고 구현할 수 있을까요??There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 이 부분에 대한 피드백을 빠트렸군요.. ㅠㅠ query parameter를 사용해서 전달하면 좀 더 좋을 것 같아요!! path parameter는 주로 리소스의 위치를 나타내는 경향이 있는데 그런 파라미터로 적절하지 않을 경우는 query parameter를 사용하면 좋은 것 같습니다