-
Notifications
You must be signed in to change notification settings - Fork 14
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
[김영찬] 1차 과제 제출 #2
base: develop
Are you sure you want to change the base?
Changes from 8 commits
1e83845
98b165e
6ab9c93
ca7ae34
d78530c
f2504b0
efdebf4
a242d08
b1df8d7
328220d
ec060c1
66cc636
5da05c9
bd94f08
e4c2671
f9d3490
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,62 +1,21 @@ | ||
package com.gildedrose; | ||
|
||
import com.gildedrose.updateLogic.UpdateLogic; | ||
import com.gildedrose.updateLogic.UpdateLogicFactory; | ||
|
||
class GildedRose { | ||
Item[] items; | ||
UpdateLogicFactory updateLogics; | ||
|
||
public GildedRose(Item[] items) { | ||
this.items = items; | ||
this.updateLogics = new UpdateLogicFactory(); | ||
} | ||
|
||
public void updateQuality() { | ||
for (int i = 0; i < items.length; i++) { | ||
if (!items[i].name.equals("Aged Brie") | ||
&& !items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) { | ||
if (items[i].quality > 0) { | ||
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) { | ||
items[i].quality = items[i].quality - 1; | ||
} | ||
} | ||
} else { | ||
if (items[i].quality < 50) { | ||
items[i].quality = items[i].quality + 1; | ||
|
||
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) { | ||
if (items[i].sellIn < 11) { | ||
if (items[i].quality < 50) { | ||
items[i].quality = items[i].quality + 1; | ||
} | ||
} | ||
|
||
if (items[i].sellIn < 6) { | ||
if (items[i].quality < 50) { | ||
items[i].quality = items[i].quality + 1; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) { | ||
items[i].sellIn = items[i].sellIn - 1; | ||
} | ||
|
||
if (items[i].sellIn < 0) { | ||
if (!items[i].name.equals("Aged Brie")) { | ||
if (!items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) { | ||
if (items[i].quality > 0) { | ||
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) { | ||
items[i].quality = items[i].quality - 1; | ||
} | ||
} | ||
} else { | ||
items[i].quality = items[i].quality - items[i].quality; | ||
} | ||
} else { | ||
if (items[i].quality < 50) { | ||
items[i].quality = items[i].quality + 1; | ||
} | ||
} | ||
} | ||
for (Item item : items) { | ||
UpdateLogic updateLogic = updateLogics.findUpdateLogic(item.name); | ||
updateLogic.update(item); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
import com.gildedrose.Item; | ||
|
||
public class BrieUpdateLogic extends UpdateLogic { | ||
@Override | ||
public void update(Item item) { | ||
decreaseSellIn(item); | ||
|
||
if (item.sellIn < 0) { | ||
increaseQuality(item, 2); | ||
} else { | ||
increaseQuality(item, 1); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
import com.gildedrose.Item; | ||
|
||
public class ConjuredUpdateLogic extends UpdateLogic { | ||
@Override | ||
public void update(Item item) { | ||
decreaseSellIn(item); | ||
|
||
decreaseQuality(item, 2); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
import com.gildedrose.Item; | ||
|
||
public class ElseUpdateLogic extends UpdateLogic { | ||
@Override | ||
public void update(Item item) { | ||
decreaseSellIn(item); | ||
|
||
decreaseQuality(item, 1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
public enum ItemCategory { | ||
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. 혹시 왜 카테고리를 따로 만드신건지 알 수 있을까요? 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. Item의 이름이 주어졌을 때, 이름 안에서 어떤 아이템인지 알아내는 문자열을 Map에 넣으려고 하는데, 그 문자열이 길이가 길다면, 사용하는 메모리가 늘어나서 좋지 않고, 가독성도 높이기 위해 카테고리를 만들었습니다! |
||
BRIE, PASS, CONJURED, SULFURAS, ELSE | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
import com.gildedrose.Item; | ||
|
||
public class PassesUpdateLogic extends UpdateLogic { | ||
@Override | ||
public void update(Item item) { | ||
decreaseSellIn(item); | ||
|
||
if (item.sellIn < 0) { | ||
resetQuality(item); | ||
} else if (item.sellIn <= 5) { | ||
increaseQuality(item, 3); | ||
} else if (item.sellIn <= 10) { | ||
increaseQuality(item, 2); | ||
} else { | ||
increaseQuality(item, 1); | ||
} | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
import com.gildedrose.Item; | ||
|
||
public class SulfurasUpdateLogic extends UpdateLogic { | ||
@Override | ||
public void update(Item item) { | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
import com.gildedrose.Item; | ||
|
||
public abstract class UpdateLogic { | ||
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. 추상 클래스로 바꿔 중복되는 코드를 잘 처리하신 것 같습니다! |
||
public static final int MAX_QUALITY = 50; | ||
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. 고정 변수도 좋은 거 같아요! |
||
public static final int MIN_QUALITY = 0; | ||
public static final int DOWN_MULTIPLE = 2; | ||
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. 혹시 enum과 private static final 중에 후자를 선택한 이유가 있으실까요? 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. 안녕하세요. 먼저, 시험이 오늘 끝난 터라 늦게 답변드린 점 죄송합니다. 치현님이 말씀해주신 enum 사용의 이유도 생각해보았지만, 개인적으로
크게 위 3가지 이유로, 후자를 선택했었습니다. |
||
|
||
protected void decreaseSellIn(Item item) { | ||
item.sellIn -= 1; | ||
} | ||
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. 추상 클래스에 공통적인 기능을 수행하는 메서드를 따로 빼두셨네요. |
||
|
||
protected void decreaseQuality(Item item, int decreaseAmount) { | ||
if (item.sellIn < 0) { | ||
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. 0 이라는 숫자가 문제에서는 "마감일까지 남은 일 수" 라고 정의되어 이것을 기준으로 분기를 했지만, |
||
item.quality = Math.max(item.quality - decreaseAmount * DOWN_MULTIPLE, MIN_QUALITY); | ||
} else { | ||
item.quality = Math.max(item.quality - decreaseAmount, MIN_QUALITY); | ||
} | ||
} | ||
|
||
protected void increaseQuality(Item item, int increaseAmount) { | ||
item.quality = Math.min(item.quality + increaseAmount, MAX_QUALITY); | ||
} | ||
|
||
protected void resetQuality(Item item) { | ||
item.quality = 0; | ||
} | ||
|
||
public abstract void update(Item item); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package com.gildedrose.updateLogic; | ||
|
||
import static com.gildedrose.updateLogic.ItemCategory.*; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
public class UpdateLogicFactory { | ||
private final Map<ItemCategory, UpdateLogic> updateLogicMap; | ||
|
||
public UpdateLogicFactory() { | ||
updateLogicMap = new HashMap<>(); | ||
|
||
updateLogicMap.put(BRIE, new BrieUpdateLogic()); | ||
updateLogicMap.put(PASS, new PassesUpdateLogic()); | ||
updateLogicMap.put(CONJURED, new ConjuredUpdateLogic()); | ||
updateLogicMap.put(SULFURAS, new SulfurasUpdateLogic()); | ||
updateLogicMap.put(ELSE, new ElseUpdateLogic()); | ||
} | ||
|
||
public UpdateLogic findUpdateLogic(String name) { | ||
if (name.contains("Aged Brie")) { | ||
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. .contains가 계속 반복 되니 반복문을 사용해도 되지 않을까요?! 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. 감사합니다! |
||
return updateLogicMap.get(BRIE); | ||
} else if (name.contains("Backstage passes")) { | ||
return updateLogicMap.get(PASS); | ||
} else if (name.contains("Conjured")) { | ||
return updateLogicMap.get(CONJURED); | ||
} else if (name.contains("Sulfuras")) { | ||
return updateLogicMap.get(SULFURAS); | ||
} else | ||
return updateLogicMap.get(ELSE); | ||
} | ||
} |
This file was deleted.
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.
변수명과 클래스명을 잘지어서 어떤 로직을 수행하는지 직관적으로 보여요