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차 과제 제출 #6

Open
wants to merge 2 commits 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
57 changes: 8 additions & 49 deletions Java/src/main/java/com/gildedrose/GildedRose.java
Original file line number Diff line number Diff line change
@@ -1,62 +1,21 @@
package com.gildedrose;

import com.gildedrose.Update.UpdateLogic;
import com.gildedrose.Update.UpdateLogicFactory;

class GildedRose {
Item[] items;
UpdateLogicFactory updateLogicFactory;

public GildedRose(Item[] items) {
this.items = items;
updateLogicFactory = 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 = updateLogicFactory.getUpdateLogic(item);
updateLogic.update(item);
}
}
}
22 changes: 22 additions & 0 deletions Java/src/main/java/com/gildedrose/Update/UpdateAgedBrieLogic.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.gildedrose.Update;

import com.gildedrose.Item;

public class UpdateAgedBrieLogic implements UpdateLogic {

@Override
public void update(Item item) {

if (item.quality < 50) {
item.quality++;
}

// 기존 시스템도 quality값 50 미만인지 검사 후 quality++ -> sellIn-- -> 판매 기한 지났는지 검사 후 quality++이므로 해당 구조 유지
item.sellIn--;

if (item.sellIn < 0 && item.quality < 50) {
item.quality++;
}

}
Copy link
Member

Choose a reason for hiding this comment

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

메서드 끝나기 전에 의미 없는 한 줄이 추가된 거 같습니다.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.gildedrose.Update;

import com.gildedrose.Item;

public class UpdateBackstagePassesLogic implements UpdateLogic {

@Override
public void update(Item item) {

if (item.quality < 50) {
item.quality++;
}
if (item.sellIn <= 10 && item.quality < 50) {
item.quality++;
}
if (item.sellIn <= 5 && item.quality < 50) {
item.quality++;
}
Comment on lines +7 to +18
Copy link

Choose a reason for hiding this comment

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

저번 세션때 피드백을 반영해서 매직 넘버로 관리하면 좋을 것 같습니다!


Copy link
Member

Choose a reason for hiding this comment

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

2배, 3배가 된다는 것을 if문으로 처리하셨군요.
이 부분 약간 이해하기 힘든 거 같습니다! 직관적이지 않아보여요!
3배가 된다는 건 * 3로직으로 변경해보시면 어떨까 싶어요

Copy link
Member Author

Choose a reason for hiding this comment

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

작성하면서도 고민했던 부분인데 말씀해주신대로 *3으로 작성하는게 가독성이 더 좋을 것 같네요 수정해보겠습니다 피드백 감사합니다!

item.sellIn--;

if (item.sellIn < 0) {
item.quality = 0;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.gildedrose.Update;

import com.gildedrose.Item;

public class UpdateConjuredItemLogic implements UpdateLogic {

@Override
public void update(Item item) {

if (item.quality > 0) {
item.quality -= 2;
}

item.sellIn--;

if (item.sellIn < 0 && item.quality > 0) {
item.quality -= 2;
}

if (item.quality < 0) {
// quality가 음수가 될수도 있으므로
// sellIn > 0, quality = 1인 경우, sellIn < 0, quality = 3인 경우 해당
Comment on lines +20 to +22
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;
Copy link
Member

@supsup-hae supsup-hae Oct 7, 2024

Choose a reason for hiding this comment

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

quality 값을 조정하는 코드들을 별도의 클래스 혹은 메소드를 통해서 관리한다면 코드 중복이 많이 줄어들 거 같아요!

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.gildedrose.Update;

import com.gildedrose.Item;

public class UpdateDefaultItemLogic implements UpdateLogic {

@Override
public void update(Item item) {
if (item.quality > 0) {
item.quality--;
}

item.sellIn--;

if (item.sellIn < 0 && item.quality > 0) {
item.quality--;
}
}
}
7 changes: 7 additions & 0 deletions Java/src/main/java/com/gildedrose/Update/UpdateLogic.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.gildedrose.Update;

import com.gildedrose.Item;

public interface UpdateLogic {
void update(Item item);
}
23 changes: 23 additions & 0 deletions Java/src/main/java/com/gildedrose/Update/UpdateLogicFactory.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.gildedrose.Update;

import com.gildedrose.Item;

public class UpdateLogicFactory {
public static UpdateLogic getUpdateLogic(Item item) {
if (item.name.contains("Conjured")) {
// Conjured 아이템인 경우 switch문에서 판별 불가능하므로 별도 처리
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

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

궁금하게 있어 여쭈어봅니다! switch문에서 conjured아이템인 경우 판별 불가능한 이유 알 수 있을까요??

Copy link
Member Author

@0-x-14 0-x-14 Oct 9, 2024

Choose a reason for hiding this comment

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

switch문의 조건은 조건에서 제시한 값과 정확히 일치하는 경우에만 실행됩니다! Conjured 아이템이 테스트 케이스에서 제시된 “Conjured Mana Cake“만 있으면 switch문으로 처리해도 상관없겠지만, 테스트 케이스 외에도 다른 Conjured 아이템까지 처리하는 방향으로 구현하였습니다.

return new UpdateConjuredItemLogic();
}

switch (item.name) {
Copy link
Member

@supsup-hae supsup-hae Oct 7, 2024

Choose a reason for hiding this comment

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

팩토리를 이용해서 아이템별 처리 로직을 구분하신 거 좋네요!
다른 아이템들도 전체 이름이 아닌 위 Conjured처럼 특정 이름을 기반으로 분류되도록 수정하면 유지보수 측면에서 더 좋을 거 같아요!

Copy link

Choose a reason for hiding this comment

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

switch문을 사용해서 가독성이 좋아진 것 같습니다!👍

case "Aged Brie":
return new UpdateAgedBrieLogic();
case "Backstage passes to a TAFKAL80ETC concert":
return new UpdateBackstagePassesLogic();
case "Sulfuras, Hand of Ragnaros":
return new UpdateSulfurasLogic();
default:
return new UpdateDefaultItemLogic();
}
}
}
11 changes: 11 additions & 0 deletions Java/src/main/java/com/gildedrose/Update/UpdateSulfurasLogic.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.gildedrose.Update;

import com.gildedrose.Item;

public class UpdateSulfurasLogic implements UpdateLogic {

@Override
public void update(Item item) {

}
}
19 changes: 9 additions & 10 deletions Java/src/test/java/com/gildedrose/TexttestFixture.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ public static void main(String[] args) {
System.out.println("OMGHAI!");

Item[] items = new Item[] {
new Item("+5 Dexterity Vest", 10, 20), //
new Item("Aged Brie", 2, 0), //
new Item("Elixir of the Mongoose", 5, 7), //
new Item("Sulfuras, Hand of Ragnaros", 0, 80), //
new Item("Sulfuras, Hand of Ragnaros", -1, 80),
new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20),
new Item("Backstage passes to a TAFKAL80ETC concert", 10, 49),
new Item("Backstage passes to a TAFKAL80ETC concert", 5, 49),
// this conjured item does not work properly yet
new Item("Conjured Mana Cake", 3, 6) };
new Item("+5 Dexterity Vest", 10, 20), //
new Item("Aged Brie", 2, 0), //
new Item("Elixir of the Mongoose", 5, 7), //
new Item("Sulfuras, Hand of Ragnaros", 0, 80), //
new Item("Sulfuras, Hand of Ragnaros", -1, 80),
new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20),
new Item("Backstage passes to a TAFKAL80ETC concert", 10, 49),
new Item("Backstage passes to a TAFKAL80ETC concert", 5, 49),
new Item("Conjured Mana Cake", 3, 6)};

GildedRose app = new GildedRose(items);

Expand Down