Skip to content
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차 과제 제출 #5

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 65 additions & 31 deletions Java/src/main/java/com/gildedrose/GildedRose.java
Original file line number Diff line number Diff line change
@@ -1,62 +1,96 @@
package com.gildedrose;

import java.util.HashMap;

import com.gildedrose.update.UpdateItem;
import com.gildedrose.update.UpdateItemBackStage;
import com.gildedrose.update.UpdateItemBrie;
import com.gildedrose.update.UpdateItemConjured;
import com.gildedrose.update.UpdateItemStandard;
import com.gildedrose.update.UpdateItemSulfuras;

class GildedRose {
Item[] items;

public GildedRose(Item[] items) {
this.items = items;
}


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;
}
HashMap<String, UpdateItem> updaters = new HashMap<>();
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());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 테스트 코드가 만약 "Conjured Mana Cake"가 아니라 "Conjured xxxx xxxx" 같이 다른 케이스가 나오게 되면 UpdateItemStandard의 update가 실행될 것 같아요. 테스트 코드가 바뀌더라도 동작할 수 있도록 고쳐보시면 좋을 것 같습니다!


UpdateItemStandard updateItemStandard = new UpdateItemStandard();

for (Item item : items) {
updaters.getOrDefault(item.name,updateItemStandard).update(item);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 getOrDefault 함수를 처음봐서 되게 참신하게 느껴졌습니다! 새로운 문법 알고 갑니다 감사합니다~~

}
}

}


/*
public void updateQuality() {

for (Item item : items) {
if (!item.name.equals("Aged Brie") // 브리치즈, 콘서트 티켓이 아닌 경우 -> 일반 아이템
&& !item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.quality > 0) {
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.quality = item.quality - 1;
} // Sulfuras 아닌 경우 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;
} else { // 브리치즈 or 콘서트 티켓인 경우
if (item.quality < 50) {
item.quality = item.quality + 1; // 가치 50 미만이면 1 증가

// 콘서트 티켓
if (item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.sellIn < 11) { // 10일 이하면 가치 총 2 증가
if (item.quality < 50) {
item.quality = item.quality + 1;
}
}

if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
if (item.sellIn < 6) { // 5일 이하면 가치 총 3 증가
if (item.quality < 50) {
item.quality = item.quality + 1;
}
}
}
}
}

if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].sellIn = items[i].sellIn - 1;
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.sellIn = item.sellIn - 1; // Surfuras는 기간 감소 x
}

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;
if (item.sellIn < 0) { // 남은 기간이 0 미만
if (!item.name.equals("Aged Brie")) {
if (!item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.quality > 0) {
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.quality = item.quality - 1; // 브리, 콘서트, Sulfuras 다 아님 -> 일반 아이템 : 총 2씩 감소
}
}
} else {
items[i].quality = items[i].quality - items[i].quality;
item.quality = item.quality - item.quality; // 콘서트 가치 0
}
} else {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
} else { // 브리치즈는 총 2씩 증가
if (item.quality < 50) {
item.quality = item.quality + 1;
}
}
}
}

}
}

}

*/
22 changes: 22 additions & 0 deletions Java/src/main/java/com/gildedrose/update/UpdateItem.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.gildedrose.update;

import com.gildedrose.Item;

public interface UpdateItem {
static final int MAX_QUALITY = 50;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상수 선언을 해서 사용하신 게 너무 좋은 것 같습니다!

static final int MIN_QUALITY = 0;

public void update(Item item);

static void decreaseSellIn(Item item) {item.sellIn -= 1;}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반복되는 작업 함수로 만드셔서 사용한 것도 너무 좋은 것 같아요~


static void increaseQuality(Item item) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반복되는 작업들을 static 메소드롤 정의하여서 하위 코드에서도 간편하게 호출하여서 사용할 수 있는 점이 좋은 거 같아요!. 코드 가독성도 좋아서 읽어보기 편했습니다!

if(item.quality < MAX_QUALITY) item.quality += 1;
}

static void decreaseQuality(Item item) {
if(item.quality > MIN_QUALITY) item.quality -= 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상수화를 하여서 값을 사용하다보니, 유지보수하기도 편할 거 같고 의미가 명확히 다가와 읽어보기 편하였습니다

}


}
19 changes: 19 additions & 0 deletions Java/src/main/java/com/gildedrose/update/UpdateItemBackStage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.gildedrose.update;

import com.gildedrose.Item;

public class UpdateItemBackStage implements UpdateItem{
@Override
public void update(Item item) {
UpdateItem.decreaseSellIn(item);

if (item.sellIn < 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드를 읽어보니 이러한 값들에 대해서도 상수화를 했으면 좋았을 거 같습니다!

item.quality = 0;
return;
}

UpdateItem.increaseQuality(item);
if(item.sellIn < 11) UpdateItem.increaseQuality(item);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 있는 11, 6도 상수로 만들면 좋을 것 같습니다!
그런데 11 6 미만으로 하는 것이 성능 상으로는 조금 더 좋을 것 같지만 가독성 측면에서는 손해인 것 같다는 생각도 들어요!
주석을 사용하거나 <= 을 사용하는 것도 나쁘지 않을 것 같습니다.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10일 이하를 나타내기 위해서 < 11로 쓰신 것 같은데, <= 10으로 쓰는 것이 더 좋은 것 같아요. 또, 제한 일수가 바뀔 것을 대비하여 상수화하는 것이 더 좋을 것 같습니다!

if(item.sellIn < 6) UpdateItem.increaseQuality(item);
}
}
15 changes: 15 additions & 0 deletions Java/src/main/java/com/gildedrose/update/UpdateItemBrie.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.gildedrose.update;

import com.gildedrose.Item;
import com.gildedrose.update.UpdateItem;

public class UpdateItemBrie implements UpdateItem {

@Override
public void update(Item item) {
UpdateItem.decreaseSellIn(item);

UpdateItem.increaseQuality(item);
if(item.sellIn < 0) UpdateItem.increaseQuality(item);
}
}
13 changes: 13 additions & 0 deletions Java/src/main/java/com/gildedrose/update/UpdateItemConjured.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.gildedrose.update;

import com.gildedrose.Item;

public class UpdateItemConjured implements UpdateItem{
@Override
public void update(Item item) {
UpdateItem.decreaseSellIn(item);

UpdateItem.decreaseQuality(item);
UpdateItem.decreaseQuality(item);
Comment on lines +10 to +11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decreaseQuality를 두 번 쓰는 방법도 있지만, 조건문을 통해 감소하는 양을 다르게 전달하거나, decreaseQuaility 내부에서 조건문을 만드는 방법도 있을 것 같아요!

}
}
12 changes: 12 additions & 0 deletions Java/src/main/java/com/gildedrose/update/UpdateItemStandard.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.gildedrose.update;

import com.gildedrose.Item;

public class UpdateItemStandard implements UpdateItem{
@Override
public void update(Item item) {
UpdateItem.decreaseSellIn(item);

UpdateItem.decreaseQuality(item);
}
}
10 changes: 10 additions & 0 deletions Java/src/main/java/com/gildedrose/update/UpdateItemSulfuras.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.gildedrose.update;

import com.gildedrose.Item;

public class UpdateItemSulfuras implements UpdateItem{
@Override
public void update(Item item) {

}
}