My Gilded Rose entry

gilded_rose.js

class Item {
  constructor (name, sellIn, quality) {
    this.name = name
    this.sellIn = sellIn
    this.quality = quality
  }
}

class Shop {
  constructor (items = []) {
    this.items = items
  }

  updateQuality () {
    // items are mutated in place!
    this.items.forEach(item => {
      // this item has fixed `quality` and `sellIn`
      if (item.name === 'Sulfuras, Hand of Ragnaros') {
        return
      }

      if (item.name === 'Aged Brie') {
        // brie improves in quality with age, twice as fast once past sell-by date
        item.quality += item.sellIn <= 0 ? 2 : 1
      } else if (item.name === 'Backstage passes to a TAFKAL80ETC concert') {
        // backstage passes improve in quality until the concert, then become worthless
        if (item.sellIn <= 0) {
          item.quality = 0
        } else {
          item.quality += item.sellIn <= 5 ? 3 : item.sellIn <= 10 ? 2 : 1
        }
      } else {
        // regular items decrease in quality with age, faster once past sell-by date
        item.quality -= item.sellIn <= 0 ? 2 : 1
      }

      // quality is clamped
      if (item.quality <= 0) {
        item.quality = 0
      }

      if (item.quality >= 50) {
        item.quality = 50
      }

      item.sellIn--
    })

    return this.items
  }
}

module.exports.Item = Item
module.exports.Shop = Shop

This is my preferred refactoring of this source file, which is the JavaScript version of the "Gilded Rose" refactoring exercise originally created by Terry Hughes.

Comments in-line are part of my refactoring and are addressed to hypothetical future maintainers of this code. Actual comments from me, the person participating in this exercise, to you, the person reviewing my submission, are as follows:

• I applied some personal formatting preferences, notably omitting semicolons.
• This refactoring preserves several assumptions which, in the original Java, are hard truths, such as the fact that quality and sellIn are integers, and that every element of items is an object of class Item.
• As you can see, I favour branching procedural code over the use of inheritance or object-oriented code.
• Both before and after refactoring, this code is riddled with magic numbers and magic strings. I've left them all where they are for now because in my opinion this maximises legibility for this specific source file, taken in isolation. As soon as any other code appeared - for displaying item properties to the shopkeeper, for example - they would need to be factored out.
• Rather than assign a brand new object to module.exports, I prefer to assign properties to the existing module.exports object. This decreases the probability of some very troublesome behaviour in the event of circular imports.
• I find it a little concerning that the updateQuality method both modifies the items in place and returns this.items. To me, returning this.items gives the false impression that the returned value is a brand new array and that the original this.items was left unchanged. (Array.prototype.sort has the same problem.) I would prefer it if updateQuality returned undefined. However, my refactoring preserves the existing behaviour.
• I also find it slightly puzzling that theoretically the "Sulfuras" item could start out with a quality outside of the closed integer range [0, 50], 60 say, and is expected to stay that way. (Again, my refactoring preserves this behaviour.) Is quality capped or not? Is this a bug in the game? Note that factoring these values out as, say, const MIN_QUALITY = 0 and const MAX_QUALITY = 50 would be wrong in this scenario. What would we even call these constants, in this case?