[남기훈] 1차 과제 제출 #1
Conversation
| } | ||
| } | ||
|
|
||
| /*public void updateQuality() { |
| class GildedRose { | ||
| Item[] items; | ||
| UpdateLogicFactory updateLogicFactory; |
There was a problem hiding this comment.
혹시 접근자 private와 제어자 final을 안 넣으신 이유가 있으실까용?
There was a problem hiding this comment.
바뀔수도 있다고 착각했습니다.. private final 넣어야 합니다..ㅎ
| logicMap.put(BACKSTAGE_CONCERT, new BackStageConcertUpdateLogic()); | ||
| logicMap.put(CONJURED, new ConjuredUpdateLogic()); | ||
| logicMap.put(OTHERS, new DefaultUpdateLogic()); | ||
| } |
There was a problem hiding this comment.
오 아이템별 처리 로직을 팩토리로 관리하는 거 좋네요!
|
|
||
| import com.gildedrose.Item; | ||
|
|
||
| public interface UpdateLogic { |
| @Override | ||
| public void update(Item item) { | ||
| if (item.sellIn <= 0) { | ||
| item.quality = Math.max(0, item.quality - 2); |
There was a problem hiding this comment.
2같은 매직넘버는 상수로 처리하면 좋을 거 같아요
https://prodo-developer.tistory.com/118
"Conjured" 아이템은 일반 아이템의 2배의 속도로 품질(Quality)이 저하됩니다.
추후 일반 아이템의 품질 저하가 하루 3씩으로 변경된다면 위 요구 사항은 어떻게 될까요?
이 또한 2 * 3으로 변경되어야 합니다.
따라서 애초에
일반 아이템의 품질 저하량, Conjured의 품질 저하 속도를 모두 전역 상수로 관리하면 추후 유지보수도 훨 좋지 않을까요?
| @@ -1,16 +1,29 @@ | |||
| package com.gildedrose; | |||
There was a problem hiding this comment.
커밋 단위를 좀 더 신경쓰면 좋을 거 같아요!
특히 맨 위 커밋은 커밋 메세지랑 내용이 많이 다른 거 같아요!
커밋 단위를 기능 단위로 조절하는 것도 좋은 방법이에요!
| this.nameList = nameList; | ||
| } | ||
|
|
||
| public static ItemClassification of(Item item) { |
There was a problem hiding this comment.
item classification을 따로 만들어서 factory와 분리해 놓은게 좋은거 같아요!
| @Override | ||
| public void update(Item item) { | ||
| if (item.sellIn <= 0) { | ||
| item.quality = Math.max(0, item.quality - 4); |
| import java.util.concurrent.ConcurrentHashMap; | ||
|
|
||
| public class UpdateLogicFactory { | ||
| private final Map<ItemClassification, UpdateLogic> logicMap = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
ConcurrentHashMap을 사용하신 특정한 이유가 있을까요?
There was a problem hiding this comment.
ConcurrentHashMap과 HashMap에 차이점은 동시성 제어의 유무에 있습니다. 멀티쓰레드 환경에서 여러 쓰레드가 하나의 공유 자원에 접근하지 못하게 하는 '쓰레드 동기화'가 필수적입니다. 기존의 HashMap은 method/block lock을 통해 감싸주던지 synchronized map을 사용해 thread-safe하게 유지시켰습니다. 하지만 하나의 Thread가 lock을 유지하는 동안 아무도 해당 Map을 사용할 수 없는 상당한 오버헤드가 발생합니다. 이를 해결하기 위한 것이 부분 lock을 사용하는 ConcurrentHashMap입니다!
제가 해당 내용을 공부했을 때 자료입니다. 동작원리랑 자세히 나와있으니 참고하면 좋을 것 같아요!
https://velog.io/@alsgus92/ConcurrentHashMap%EC%9D%98-Thread-safe-%EC%9B%90%EB%A6%AC
| updateLogic.update(item); | ||
| } | ||
| } | ||
|
|
| public class AgedBrieUpdateLogic implements UpdateLogic { | ||
| @Override | ||
| public void update(Item item) { | ||
| if (item.quality < 50) { |
There was a problem hiding this comment.
50, 0 같은 매직넘버는 상수로 관리해도 좋을 것 같아요
| class GildedRose { | ||
| Item[] items; | ||
| UpdateLogicFactory updateLogicFactory; |
No description provided.