Refactoring with JS: FCC daily - Blood Bank

2026-06-27

Today, i was having fun solving this daily challenge by FreeCodeCamp,

Blood Bank

This is my initial solution, thinking greedily...

O bloodType is universal donor, but sadly only receives donation from O
therefore priority of blood O should be given to patient of O
then patient A should take blood A first, and only after that, take blood O
same goes to patient B, should take blood B first, then take blood O
lastly, patient AB is universal receipient, one can take any kind of blood

function triageBlood(bank, patients) {

  const totalPatient = patients.length
  const bankCount = getCount(bank)
  const patientCount = getCount(patients)

  let res = 0
  // priority 
  // check O patient first, only receive O
  let smaller = Math.min(patientCount['O'], bankCount['O'])
  res += smaller
  bankCount['O'] -= smaller
  patientCount['O'] -= smaller

  // then check A , only receive A or O
  smaller = Math.min(patientCount['A'], bankCount['A'])
  res += smaller
  bankCount['A'] -= smaller
  patientCount['A'] -= smaller

  smaller = Math.min(patientCount['A'], bankCount['O'])
  res += smaller
  bankCount['O'] -= smaller
  patientCount['A'] -= smaller

  // then check B , use B or O
  smaller = Math.min(patientCount['B'], bankCount['B'])
  res += smaller
  bankCount['B'] -= smaller

  smaller = Math.min(patientCount['B'], bankCount['O'])
  res += smaller
  bankCount['B'] -= smaller

  // lastly check AB
  let balance = Object.values(bankCount).reduce((s, x) => s + x, 0)
  smaller = Math.min(patientCount['AB'], balance)
  res += smaller


  return `${res} of ${totalPatient} patients served`;
}

const getCount = (arr) => {
  let count = {}
  for (let c of arr) {
    count[c] ??= 0
    count[c] += 1
  }
  count.A ??= 0
  count.B ??= 0
  count.AB ??= 0
  count.O ??= 0
  return count
}

This solution works, and would pass the FCC test cases.

But, it's so unpolished and i don't like it.

Why do i think so ? well, it's about Maintainability

Structure
- its not organized, one has to read the whole 40 lines of code figure out, and new joiner would definitely hesitates to touch any line of it, if i have to spend more than 1 minutes to figure 1 function, it's probably same for every other, therefore you could think of how many time wasted here.
Duplication
- squinting my eyes, i saw a lot of "Math.min", "-= smaller"
Abstaction
- I only saw 1 function aka getCount. Other than that, whole logic is inside 1 big block. It lacks of meaningful naming and sub-function, so i have to read the comment to figure out and hidden rule.

Then i realized, with FCC prepared all the test cases, this is another good exercise again, as a disciple of clean coder.

In his book (Refactoring 2018), Martin suggested starting to break big chunks of codes into tiny function. Ok, lets go.

function triageBlood(bank, patients) {

  const totalPatient = patients.length
  const bankCount = getCount(bank)
  const patientCount = getCount(patients)

  let res = 0

  res += distributeO(patientCount, bankCount)
  res += distributeA(patientCount, bankCount)
  res += distributeB(patientCount, bankCount)
  res += distributeAB(patientCount, bankCount)

  return `${res} of ${totalPatient} patients served`;
}

const distributeO = (patientCount, bankCount) => {
  // priority 
  // check O patient first, only receive O
  let smaller = Math.min(patientCount['O'], bankCount['O'])
  bankCount['O'] -= smaller
  patientCount['O'] -= smaller
  return smaller
}

const distributeA = (patientCount, bankCount) => {
  // then check A , only receive A or O
  let patientMatched = 0
  let smaller = Math.min(patientCount['A'], bankCount['A'])
  patientMatched += smaller
  bankCount['A'] -= smaller
  patientCount['A'] -= smaller

  smaller = Math.min(patientCount['A'], bankCount['O'])
  patientMatched += smaller
  bankCount['O'] -= smaller
  patientCount['A'] -= smaller

  return patientMatched
}

const distributeB = (patientCount, bankCount) => {
  // then check B , use B or O
  let patientMatched = 0
  let smaller = Math.min(patientCount['B'], bankCount['B'])
  patientMatched += smaller
  bankCount['B'] -= smaller

  smaller = Math.min(patientCount['B'], bankCount['O'])
  patientMatched += smaller
  bankCount['B'] -= smaller

  return patientMatched
}

const distributeAB = (patientCount, bankCount) => {
  // lastly check AB
  let balance = Object.values(bankCount).reduce((s, x) => s + x, 0)
  let smaller = Math.min(patientCount['AB'], balance)
  return smaller
}

const getCount = (arr) => {
  let count = {}
  for (let c of arr) {
    count[c] ??= 0
    count[c] += 1
  }
  count.A ??= 0
  count.B ??= 0
  count.AB ??= 0
  count.O ??= 0
  return count
}

Now we have more total lines

Each function has a meaningful name

And main function act like a upper layer controller, abstraction of "distribute something" is being extracted.

But, something is not right here

distributeO = (patientCount, bankCount)
distributeA = (patientCount, bankCount)
distributeB = (patientCount, bankCount)
distributeAB = (patientCount, bankCount)

these function, from perspective of pure function, is a Big NO NO.

it modifies the incoming hashmap/object of patientCount, bankCount

It troubles me here, why do i have to keep passing these patientCount & bankCount to each distribute function, and since each fn somehow need to modify the data, or the states, is there a better design ?

Waiiit, did i just mentioned "states" ?

If it's a state related, then a OOP is the way. Lets refactor it to OOP way.

function triageBlood(bank, patients) {

  let matcher = new BloodBankMatcher(bank, patients)

  matcher.match()

  return `${matcher.totalMatched} of ${patients.length} patients served`;
}

class BloodBankMatcher {

  constructor(bank, patients) {
    this.bankCount = this.getCount(bank)
    this.patientCount = this.getCount(patients)
    this.totalMatched = 0
  }

  match() {
    this.totalMatched += this.distributeO(this.patientCount, this.bankCount)
    this.totalMatched += this.distributeA(this.patientCount, this.bankCount)
    this.totalMatched += this.distributeB(this.patientCount, this.bankCount)
    this.totalMatched += this.distributeAB(this.patientCount, this.bankCount)
  }

  distributeO = (patientCount, bankCount) => {
    // priority 
	// check O patient first, only receive O
	let smaller = Math.min(patientCount['O'], bankCount['O'])
	bankCount['O'] -= smaller
	patientCount['O'] -= smaller
	return smaller
  }

  distributeA = (patientCount, bankCount) => {
	// then check A , only receive A or O
	let patientMatched = 0
	let smaller = Math.min(patientCount['A'], bankCount['A'])
	patientMatched += smaller
	bankCount['A'] -= smaller
	patientCount['A'] -= smaller

	smaller = Math.min(patientCount['A'], bankCount['O'])
	patientMatched += smaller
	bankCount['O'] -= smaller
	patientCount['A'] -= smaller

	return patientMatched
  }

  distributeB = (patientCount, bankCount) => {
	// then check B , use B or O
	let patientMatched = 0
	let smaller = Math.min(patientCount['B'], bankCount['B'])
	patientMatched += smaller
	bankCount['B'] -= smaller

	smaller = Math.min(patientCount['B'], bankCount['O'])
	patientMatched += smaller
	bankCount['B'] -= smaller

	return patientMatched
  }

  distributeAB = (patientCount, bankCount) => {
	// lastly check AB
	let balance = Object.values(bankCount).reduce((s, x) => s + x, 0)
	let smaller = Math.min(patientCount['AB'], balance)
	return smaller
  }

  getCount = (arr) => {
    let count = {
      A: 0,
      B: 0,
      AB: 0,
      O: 0,
    }
    arr.forEach(type => count[type] += 1)
    return count
  }

}

Great, here i created a new matcher class, which i can wrote bunch of simple unit tests if i want. But let's not do that now.

Mind you, when i refactored each step, i always check against the test cases, to ensure the IDE happy with the syntax while i didn't mess with business logic

Why passing internal state when u can access it directly with OOP -> "this."

So, further refactoring it to address the unncessary passing of this as function argument.

function triageBlood(bank, patients) {

  let matcher = new BloodBankMatcher(bank, patients)

  matcher.match()

  return `${matcher.totalMatched} of ${patients.length} patients served`;
}

class BloodBankMatcher {

  constructor(bank, patients) {
    this.bankCount = this.getCount(bank)
    this.patientCount = this.getCount(patients)
    this.totalMatched = 0
  }

  match() {
    this.totalMatched += this.distributeO()
    this.totalMatched += this.distributeA()
    this.totalMatched += this.distributeB()
    this.totalMatched += this.distributeAB()
  }

  distributeO = () => {
    // priority 
	// check O patient first, only receive O
	let smaller = Math.min(this.patientCount['O'], this.bankCount['O'])
	this.bankCount['O'] -= smaller
	this.patientCount['O'] -= smaller
	return smaller
  }

  distributeA = () => {
	// then check A , only receive A or O
	let patientMatched = 0
	let smaller = Math.min(this.patientCount['A'], this.bankCount['A'])
	patientMatched += smaller
	this.bankCount['A'] -= smaller
	this.patientCount['A'] -= smaller

	smaller = Math.min(this.patientCount['A'], this.bankCount['O'])
	patientMatched += smaller
	this.bankCount['O'] -= smaller
	this.patientCount['A'] -= smaller

	return patientMatched
  }

  distributeB = () => {
	// then check B , use B or O
	let patientMatched = 0
	let smaller = Math.min(this.patientCount['B'], this.bankCount['B'])
	patientMatched += smaller
	this.bankCount['B'] -= smaller

	smaller = Math.min(this.patientCount['B'], this.bankCount['O'])
	patientMatched += smaller
	this.bankCount['B'] -= smaller

	return patientMatched
  }

  distributeAB = () => {
	// lastly check AB
	let balance = Object.values(this.bankCount).reduce((s, x) => s + x, 0)
	let smaller = Math.min(this.patientCount['AB'], balance)
	return smaller
  }

  getCount = (arr) => {
    let count = {
      A: 0,
      B: 0,
      AB: 0,
      O: 0,
    }
    arr.forEach(type => count[type] += 1)
    return count
  }

}

Great, can't recall from which book, but the author did suggest the recommended argument pass to a function/method should capped at 2 at max.

function() # best
function(argumentA) # good
function(argumentA, argumentB) # accepatble
function(argumentA, argumentB, argumentC, argumentD, argumentE, argumentF) # you gotta be kidding me

This refactoring keeped it less than 2, great.

So, what else work leftover? oh ya... the duplication of Math.min

Since this distributeO , distributeA, distributeB, all doing samething, comparing the values, deduct the value from count, perhaps i could refactor to a new method.

function triageBlood(bank, patients) {

  let matcher = new BloodBankMatcher(bank, patients)

  matcher.match()

  return `${matcher.totalMatched} of ${patients.length} patients served`;
}

class BloodBankMatcher {

  constructor(bank, patients) {
    this.bankCount = this.getCount(bank)
    this.patientCount = this.getCount(patients)
    this.totalMatched = 0
  }

  match() {
    this.distributeO()
    this.distributeA()
    this.distributeB()
    this.distributeAB()
  }

  distribute = (pType, bType) => {
    let smaller = Math.min(this.patientCount[pType], this.bankCount[bType])
    this.patientCount[pType] -= smaller
    this.bankCount[bType] -= smaller
    this.totalMatched += smaller
  }


  distributeO = () => {
    // O patient, only receive O
    this.distribute("O", "O")
  }

  distributeA = () => {
    // A patient only receive A or O
    // A receives from A
    this.distribute("A", "A")
    // A receives from O
    this.distribute("A", "O")
  }

  distributeB = () => {
    // B patient only receive B or O
    // B receives from B
    this.distribute("B", "B")
    // B receives from O
    this.distribute("B", "O")
  }

  distributeAB = () => {
    // lastly check AB
    let balance = Object.values(this.bankCount).reduce((s, x) => s + x, 0)
    let smaller = Math.min(this.patientCount['AB'], balance)
    this.totalMatched += smaller
  }

  getCount = (arr) => {
    let count = {
      A: 0,
      B: 0,
      AB: 0,
      O: 0,
    }
    arr.forEach(type => count[type] += 1)
    return count
  }

}

What has changed?

a new method distribute(patientBloodType, bankBloodType) appears
it modifies the this.totalMatched,
so method of match() now acts like a controller, removed the redundant this.totalMatched +=
this.totalMatched += goes into method of distributeAB

The duplication is now gone, and it looks super clean and pretty

So what else work ? I think its the spontaneous naming i had given, and some use of JS syntax wwhich would confuse the beginner, ok lets improve them.

function triageBlood(bank, patients) {

	let matcher = new BloodBankMatcher(bank, patients)

	matcher.match()

	return `${matcher.totalMatched} of ${patients.length} patients served`;
}

class BloodBankMatcher {

	constructor(bank, patients) {
		this.bankCount = this.getCount(bank)
		this.patientCount = this.getCount(patients)
		this.totalMatched = 0
	}

	match() {
		this.distributeO()
		this.distributeA()
		this.distributeB()
		this.distributeAB()
	}

	findMatchCnt = (patient, blood) => {
		return Math.min(this.patientCount[patient], this.bankCount[blood])
	}


	distribute = (pType, bType) => {
		const matchCnt = this.findMatchCnt(pType, bType)
		this.patientCount[pType] -= matchCnt
		this.bankCount[bType] -= matchCnt
		this.totalMatched += matchCnt
	}

	distributeO = () => {
		// O patient, only receive O
		this.distribute("O", "O")
	}

	distributeA = () => {
		// A patient only receive A or O
		// A receives from A
		this.distribute("A", "A")
		// A receives from O
		this.distribute("A", "O")
	}

	distributeB = () => {
		// B patient only receive B or O
		// B receives from B
		this.distribute("B", "B")
		// B receives from O
		this.distribute("B", "O")
	}

	distributeAB = () => {
		// AB patient takes any blood kind which is remaining
		const remaining = this.getRemaining()
		const matchCnt = Math.min(this.patientCount['AB'], remaining)
		this.totalMatched += matchCnt
	}

	getRemaining = () => {
		return Object.values(this.bankCount).reduce((s, x) => s + x, 0)
	}

	getCount = (arr) => {
		let count = {
			A: 0,
			B: 0,
			AB: 0,
			O: 0,
		}
		arr.forEach(type => count[type] += 1)
		return count
	}
}

Above is the final version.

Solving problem may took 15~30mins.

This refactoring definitely took longer, for me it was about 30min~45mins.

Why bother your time with this ? Because as programmers, we love maintainability and cleanliness.

I'm very satisfied with it.

Thanks for reading.