[정찬민] 1차 과제 제출 #13
[정찬민] 1차 과제 제출 #13
Conversation
chanmin-00
commented
Oct 3, 2024
chanmin-00
commented
- GildedRose 클래스 : Item Upadate 함수 추가 및 GildedRoseItem 초기화 함수 추가
- GildedRoseItem 추상 클래스 생성 후 상속 구조에 따른 개별 update 함수 구현
|
잘 읽었습니다! 저와는 다른 접근 방식으로 추상 클래스를 이용하여 문제를 해결하신 게 인상 깊었습니다! 👍 |
| } | ||
|
|
||
| private GildedRoseItem getGildedRoseItem(Item item) { | ||
| switch (item.name) { |
There was a problem hiding this comment.
아이템의 전체 이름이 아닌 특정 이름을 기반으로 아이템이 분류되도록 수정하면 유지보수 측면에서 더 좋을 거 같아요!
There was a problem hiding this comment.
그런 부분을 활용하면 좀 더 세밀한 분류가 가능할거 같네요!. 참고하겠습니다!
| decreaseSellIn(); | ||
|
|
||
| if (gildedRoseItem.sellIn < 0) // 판매 일수가 0보다 작은 경우 품질 2배 증가 | ||
| increaseQuality(1); |
There was a problem hiding this comment.
조건문 내부가 한 줄이더라도 중괄호를 사용하시는 걸 지향 드려요!
별도로 코드 추가 시 결국 중괄호를 추가해야 하며, 해당 과정에서 실수가 일어날 가능성이 높아진다고 합니다!
There was a problem hiding this comment.
Oracle의 코드 컨벤션 (7-4)을 참고해보시면 좋을 것 같습니다
https://www.oracle.com/technetwork/java/codeconventions-150003.pdf
There was a problem hiding this comment.
코드 길이를 간소화하는점에만 집중해서 해당 부분은 신경을 쓰지 못했네요. 참고해서 코드리팩토링 하겠습니다. 좋은 자료 감사합니다!
| return items; | ||
| } | ||
|
|
||
| private GildedRoseItem getGildedRoseItem(Item item) { |
There was a problem hiding this comment.
저는 객체 생성 로직을 비즈니스 로직과 분리하면 더 유지보수 측면에서 좋을거라 생각합니다. Item에 해당하는 서브 클래스를 반환하는 getFlidedRoseItem 메소드를 비즈니스 로직에 작성한 이유가 궁금합니다.
|
|
||
| import com.gildedrose.Item; | ||
|
|
||
| public abstract class GildedRoseItem { |
There was a problem hiding this comment.
추상 클래스를 사용해 공통적인 로직을 반복하지 않아도 되게 코드를 짠 것이 인상깊었습니다
| protected void decreaseQuality(int quality) { | ||
| if (gildedRoseItem.quality > 0) { | ||
| gildedRoseItem.quality = Math.max(gildedRoseItem.quality - quality, 0); | ||
| } | ||
| } | ||
|
|
||
| protected void increaseQuality(int quality) { | ||
| if (gildedRoseItem.quality < 50) { | ||
| gildedRoseItem.quality = Math.min(gildedRoseItem.quality + quality, 50); | ||
| } | ||
| } |
There was a problem hiding this comment.
파라미터로 증가, 감소하는 값을 받아 유연성 있게 코드를 짤 수 있어 인상깊었습니다. 다만 파라미터 명을 quality 로 하는 것보다 decrement나 amount 같이 감소, 증가하는 "양"이라는 점을 나타내면 더 좋을 것 같습니다
| increaseQuality(1); | ||
| decreaseSellIn(); | ||
|
|
||
| if (gildedRoseItem.sellIn < 0) // 판매 일수가 0보다 작은 경우 품질 2배 증가 |
| resetQuality(); | ||
| } else { | ||
| increaseQuality(1); | ||
| if (gildedRoseItem.sellIn < 11) |
There was a problem hiding this comment.
10 이하로 표현하는게 11 보다 읽기 쉬울 것 같습니다!
| return new BackstagePasses(item); | ||
| case "Sulfuras, Hand of Ragnaros": | ||
| return new Sulfuras(item); | ||
| case "Conjured Mana Cake": |
There was a problem hiding this comment.
Mana cake 말고도 Conjured가 들어간 모든 name에 적용해줘야하지 않을까요?!
There was a problem hiding this comment.
if문 말고 switch 문으로 객체를 생성한 점이 좋은 것 같습니다!
| protected void decreaseQuality(int quality) { | ||
| if (gildedRoseItem.quality > 0) { | ||
| gildedRoseItem.quality = Math.max(gildedRoseItem.quality - quality, 0); | ||
| } | ||
| } |
There was a problem hiding this comment.
max를 사용해서 음수 값을 0으로 처리한 것이 인상깊었습니다!
| public class Sulfuras extends GildedRoseItem { | ||
|
|
||
| public Sulfuras(Item item) { | ||
| super(item); | ||
| } | ||
|
|
||
| @Override | ||
| public void update() { | ||
| // Sulfuras는 판매 일수나 품질이 변하지 않음 | ||
| } | ||
|
|
||
| public void init() { | ||
| setQuality(80); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
init 함수로 Sulfuras 아이템의 quality를 고정시켜 놓은점이 좋았습니다!
| public void init() { | ||
| setQuality(80); | ||
| } |
There was a problem hiding this comment.
저도 이 부분에서 아이템 퀄리티를 초기화하신 점이 인상깊었습니다!
