0

I try to do a minor refactoring, but it breaks all tests.

I have a lot of ifs that I'd like to get rid off by using a jump table.

I want to go from here :


export class Pharmacy {
  constructor(drugs = []) {
    this.drugs = drugs
  }

  updatePharmacyBenefits() {
    this.drugs.forEach((drug) => {
      if (drug.name !== 'Magic Pill')
        drug.setBenefitStrategy(new MagicPillStrategy())
      if (drug.name === 'Herbal Tea')
        drug.setBenefitStrategy(new HerbalTeaStrategy())
      if (drug.name === 'Fervex')
        drug.setBenefitStrategy(new FervexStrategy())
      if (drug.name === 'Dafalgan')
        drug.setBenefitStrategy(new DafalganStrategy())

      drug.updateBenefit()
      drug.updateExpiration()
    })

    return this.drugs
  }
}

to here :

export class Pharmacy {
  constructor(drugs = []) {
    this.drugs = drugs
  }

  updatePharmacyBenefits() {
    const drugStrategies = {
      'Herbal Tea': new HerbalTeaStrategy(),
      'Magic Pill': new MagicPillStrategy(),
      Fervex: new FervexStrategy(),
      Dafalgan: new DafalganStrategy(),
    }
    const specialsDrugs = ['Herbal Tea', 'Fervex', 'Magic Pill', 'Dafalgan']

    this.drugs.forEach((drug) => {
      if (drug.name.includes(specialsDrugs))
        drug.setBenefitStrategy(drugStrategies[drug.name])

      drug.updateBenefit()
      drug.updateExpiration()
    })

    return this.drugs
  }
}

I have no meaningful errors messages, I can just observe all my tests failing has if nothing has happened.

A Mehmeto
  • 1,594
  • 3
  • 22
  • 37
  • 3
    you need to change `drug.name.includes(specialsDrugs)` to `specialDrugs.includes(drug.name)` – pilchard Jan 01 '21 at 13:54
  • 2
    You're also not accounting for "not equal to" in your refactored version: `if (drug.name !== 'Magic Pill')` – Nick Parsons Jan 01 '21 at 13:56
  • 2
    I also would suggest refactoring `const specialsDrugs = ['Herbal Tea', 'Fervex', 'Magic Pill', 'Dafalgan']` into `const specialDrugs = Object.keys(drguStrategies)` in order to avoid any mistake in the future – luckongas Jan 01 '21 at 14:10

0 Answers0