-2

Goal: I am learning about SOLID principles and trying to refactor Gilded Rose code, in order to make it fresh and clean.

What I have done: I have created an AbstractItem class as follows, which "wraps" Item class:

public abstract class AbstractItem {
    protected static final int MIN_QUALITY = 0;
    protected static int MAX_QUALITY = 50;
    public Item item;  // name, quality and sellIn
public AbstractItem(Item item) {
    this.item = item;
}

public abstract void updateQuality();

public boolean isValid(int quality) {
    return quality > MIN_QUALITY && quality < MAX_QUALITY;
}

public void updateSellIn() {
    item.sellIn--;
}

public boolean hasExpired() {
    return item.sellIn < 0;
}

public int getRate() {
    return hasExpired() ? 2 : 1;
}

}

and it gets extended by AgedBrie, Sulfuras, BackstagePasses and any other item which is sold by the Gilded Rose. For example, if Gilded Rose will sell MagicApples in 2050, I will define the MagicApples class which extends AbstractItem.

My doubt: I was wondering whether this class violates the SRP (or any other SOLID principles) or you think this is ugly or not:

public class BackstagePasses extends AbstractItem {
public BackstagePasses(Item item) {
    super(item);
}

@Override
public void updateQuality() {
    if (isValid(item.quality)) {
        if (hasExpired())
            item.quality = 0;
        else
            item.quality += getRate();
    }
}

@Override
public int getRate() {
    int daysToConcert = item.sellIn;
    // should I refactor the following statements?
    if (daysToConcert <= 5)
        return 3;
    else if (daysToConcert <= 10)
        return 2;
    else
        return 1;
}

}

Thank you for your advice and patience.

tail
  • 101
  • 1
    Single Responsibility (along with the other SOLID principles) are all entirely context-dependent. A Responsibility is something which is up to you to decide on a case-by-case basis by understanding your functional/behavioural requirements as well as the different needs and expectations of the people who ultimately decide those requirements (for example, different users, product owners, stakeholders, people who use/support the system operationally, etc). The only people who can make decisions about where to draw lines of responsibility are those who understand the project at a deeper level – Ben Cottrell Jan 09 '23 at 13:13
  • Worth reading Uncle Bob's blog which explains this in more words: https://blog.cleancoder.com/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html – Ben Cottrell Jan 09 '23 at 13:16

1 Answers1

0

Yes, it obviously has two responsibilities.

  1. Updating the quality
  2. Determining the Rate

Make getRate private.

Ewan
  • 75,506