Conversation
f-lab-seb
left a comment
f-lab-seb
left a comment
There was a problem hiding this comment.
코멘트내용 모두 추후 반영되어도 무관하다 생각하여 approve 합니다.
찬찬히 읽어봐 주시면 감사하겠습니다.
잘 작업해주셨듯 실무 DB 레벨에서 foreign key 제약조건을 의도적으로 사용하지 않는 경우가 많은데요! 이미 알고계실 수 있지만 왜 그런지 한 번 고민해봐주시면 좋을 것 같습니다. (장/단이 있을 것 같아요.)
테스트 코드는 이렇게 보니까 가독성이 참 좋아보이는 거 같기도 하고, 너무 장황해지나 싶기도 하고 저도 판단하지 못한상태라 새로운 것 써보시는 게 제게도 도움이 됩니다.
감사합니다 고생하셨습니다!
| var description: String?, | ||
| var basePrice: Int, | ||
| var categoryId: Long, | ||
| var isActive: Boolean, |
There was a problem hiding this comment.
저는 변수명에 is prefix 를 선호하지 않습니다.
Java Bean 규약에 따라 boolean getter 가 is 로 시작하는 점 때문에 jackson(ser/deser) 이나 mapper, binding 등 is prefix 변수로 인한 예상치 못한 동작을 많이 경험했습니다.
activated, active, enabled 등 다른 변수명을 사용해보시는 것을 추천드리고,
메서드에서만 is prefix 를 붙이는 것을 권장드립니다!
There was a problem hiding this comment.
자바에서는 보통 active와 같이 변수명을 사용했었는데 코틀린에서는 프로퍼티라는 개념 때문에 isActive처럼 이름을 짓게 되었습니다. 여기까지가 제 생각의 흐름이었고, 말씀해주신 부분들에 대해 생각해보지 못했어서 직접 테스트해보고 정리해보았습니다.
ObjectMapper로 테스트해보니, 프로퍼티 이름을 isActive로 지정할 경우 직렬화 시에는 "active"로 변환되고, 역직렬화할 때도 "active"라는 키가 있어야 정상적으로 매핑이 되는 것을 확인했습니다. "isActive"라는 키로 역직렬화를 시도하면 매핑이 되지 않는 것도 확인했습니다.
정리하자면, Jackson은 기본적으로 자바빈 규약을 따라 getter를 통해 직렬화를 하고 setter를 통해 역직렬화를 처리합니다. isActive처럼 이름을 짓는 경우 내부적으로 getter는 isActive(), setter는 setActive()로 생성되는데, Jackson은 이 is와 set을 prefix로 인식하여 실제 json key를 "active"로 처리합니다.
코드에는 isActive로 되어 있어도, 직렬화/역직렬화 시에는 "active"라는 이름으로 처리되어 예상치 못한 상황이 발생할 수 있을 것 같습니다.
@JsonProperty를 사용해 명시적으로 이름을 지정할 수도 있지만, isActive라는 이름을 쓰기 위해 이렇게까지 할 필요는 없어 보이고, active로 지정하면 직렬화, 역직렬화 모두 별 문제 없이 사용할 수 있을 것 같습니다.
| fun increaseStock(amount: Int) { | ||
| this.stock += amount | ||
| } | ||
|
|
||
| fun decreaseStock(amount: Int) { | ||
| require(amount <= this.stock) { "재고가 부족합니다." } | ||
| this.stock -= amount | ||
| } |
There was a problem hiding this comment.
race condition 상황에서 정상적으로 동작할지 한 번 고민해주시면 좋을 것 같아요~
There was a problem hiding this comment.
별도의 조치 없이 그대로 사용하면 다음과 같이 lost update가 발생할 것 같습니다.
그러나 락을 사용해 임계영역 내 조회와 업데이트가 원자적으로 수행되면 괜찮을 것 같습니다.
분산락 또는 간단하게 DB락으로 구현이 가능할 것 같습니다.
Redis의 단일 스레드를 활용하거나, 다음과 같이 update 쿼리에서 조회도 함께 한번의 쿼리로 원자적으로 처리하여 별도의 락 걸지 않고 해당 쿼리 중에만 짧게 배타락이 사용되도록 임계 영역을 극소화하는 것도 방법인 것 같습니다.
UPDATE product_item
SET stock = stock - :amount
WHERE id = :itemId AND stock >= :amount;그리고 배타락의 범위가 어떻든 ProductItem의 로우 단위로 배타 락이 걸린다는 것을 생각하면 ProductItem 조회에 영향이 없도록 stock을 별도 테이블로 분리하는 것도 좋은 개선 방안인 것 같습니다.
그런데 이러한 최적화로 인해 도메인 응집도가 떨어질 수 있다는 것이 트레이드 오프인 것 같다는 생각이 듭니다
|
2 participants
Closes #1