[김채은] 1차 과제 제출 #5
Conversation
| import com.gildedrose.Item; | ||
|
|
||
| public interface UpdateItem { | ||
| static final int MAX_QUALITY = 50; |
There was a problem hiding this comment.
상수 선언을 해서 사용하신 게 너무 좋은 것 같습니다!
|
|
||
| public void update(Item item); | ||
|
|
||
| static void decreaseSellIn(Item item) {item.sellIn -= 1;} |
There was a problem hiding this comment.
반복되는 작업 함수로 만드셔서 사용한 것도 너무 좋은 것 같아요~
| } | ||
|
|
||
| UpdateItem.increaseQuality(item); | ||
| if(item.sellIn < 11) UpdateItem.increaseQuality(item); |
There was a problem hiding this comment.
여기 있는 11, 6도 상수로 만들면 좋을 것 같습니다!
그런데 11 6 미만으로 하는 것이 성능 상으로는 조금 더 좋을 것 같지만 가독성 측면에서는 손해인 것 같다는 생각도 들어요!
주석을 사용하거나 <= 을 사용하는 것도 나쁘지 않을 것 같습니다.
There was a problem hiding this comment.
10일 이하를 나타내기 위해서 < 11로 쓰신 것 같은데, <= 10으로 쓰는 것이 더 좋은 것 같아요. 또, 제한 일수가 바뀔 것을 대비하여 상수화하는 것이 더 좋을 것 같습니다!
| UpdateItemStandard updateItemStandard = new UpdateItemStandard(); | ||
|
|
||
| for (Item item : items) { | ||
| updaters.getOrDefault(item.name,updateItemStandard).update(item); |
There was a problem hiding this comment.
제가 getOrDefault 함수를 처음봐서 되게 참신하게 느껴졌습니다! 새로운 문법 알고 갑니다 감사합니다~~
| updaters.put("Aged Brie", new UpdateItemBrie()); | ||
| updaters.put("Backstage passes to a TAFKAL80ETC concert", new UpdateItemBackStage()); | ||
| updaters.put("Sulfuras, Hand of Ragnaros", new UpdateItemSulfuras()); | ||
| updaters.put("Conjured Mana Cake", new UpdateItemConjured()); |
There was a problem hiding this comment.
여기서 테스트 코드가 만약 "Conjured Mana Cake"가 아니라 "Conjured xxxx xxxx" 같이 다른 케이스가 나오게 되면 UpdateItemStandard의 update가 실행될 것 같아요. 테스트 코드가 바뀌더라도 동작할 수 있도록 고쳐보시면 좋을 것 같습니다!
|
|
||
| static void decreaseSellIn(Item item) {item.sellIn -= 1;} | ||
|
|
||
| static void increaseQuality(Item item) { |
There was a problem hiding this comment.
반복되는 작업들을 static 메소드롤 정의하여서 하위 코드에서도 간편하게 호출하여서 사용할 수 있는 점이 좋은 거 같아요!. 코드 가독성도 좋아서 읽어보기 편했습니다!
| } | ||
|
|
||
| static void decreaseQuality(Item item) { | ||
| if(item.quality > MIN_QUALITY) item.quality -= 1; |
There was a problem hiding this comment.
상수화를 하여서 값을 사용하다보니, 유지보수하기도 편할 거 같고 의미가 명확히 다가와 읽어보기 편하였습니다
| public void update(Item item) { | ||
| UpdateItem.decreaseSellIn(item); | ||
|
|
||
| if (item.sellIn < 0) { |
There was a problem hiding this comment.
코드를 읽어보니 이러한 값들에 대해서도 상수화를 했으면 좋았을 거 같습니다!
| UpdateItem.decreaseQuality(item); | ||
| UpdateItem.decreaseQuality(item); |
There was a problem hiding this comment.
decreaseQuality를 두 번 쓰는 방법도 있지만, 조건문을 통해 감소하는 양을 다르게 전달하거나, decreaseQuaility 내부에서 조건문을 만드는 방법도 있을 것 같아요!
No description provided.