Nucleotide count in Scala

Question

This is my second day in learning Scala and I still need to develop a taste of functional programming, I often find myself doing imperative coding. Below is the result of my TDD practice.

Code

class DNA(dnaSequence: String) {
  var count = collection.mutable.Map('A' -> 0, 'T' -> 0, 'C' -> 0, 'G' -> 0)

  dnaSequence.foreach { (i: Char) =>
    validate(i)
    count = count.updated(i, count.get(i).getOrElse(0) + 1)
  }

  def nucleotideCounts() = count

  def nucleotideCounts(nucleotide: Char) = {
    validate(nucleotide)
    count.get(nucleotide).getOrElse(0)
  }

  private def validate(chr: Char) = {
    if (!count.contains(chr)) throw new RuntimeException()
  }
}

Test Suite

import org.scalatest._

class NucleotideCountSpecs extends FlatSpec with Matchers {
  "empty dna string" should "have no adenine" in {
    new DNA("").nucleotideCounts('A') should be (0)
  }

  it should "have no nucleotides" in {
    val expected = Map('A' -> 0, 'T' -> 0, 'C' -> 0, 'G' -> 0)
    new DNA("").nucleotideCounts should be (expected)
  }

  "a repetitive sequence" should "count cytosine" in {
    new DNA("CCCCC").nucleotideCounts('C') should be (5)
  }

  it should "have only guanine" in {
    val expected = Map('A' -> 0, 'T' -> 0, 'C' -> 0, 'G' -> 8)
    new DNA("GGGGGGGG").nucleotideCounts should be (expected)
  }

  "a mixed dna string" should "count only thymine" in {
    new DNA("GGGGGTAACCCGG").nucleotideCounts('T') should be (1)
  }

  it should "count a nucleotide only once" in {
    val dna = new DNA("CGATTGGG")
    dna.nucleotideCounts('T')
    dna.nucleotideCounts('T') should be (2)
  }

  it should "not change counts after counting adenine" in {
    val dna = new DNA("GATTACA")
    dna.nucleotideCounts('A')
    val expected = Map('A' -> 3, 'T' -> 2, 'C' -> 1, 'G' -> 1)
    dna.nucleotideCounts should be (expected)
  }

  it should "validate nucleotides" in {
    a [RuntimeException] should be thrownBy new DNA("GACT").nucleotideCounts('X')
  }

  it should "validate dna not rna" in {
    a [RuntimeException] should be thrownBy new DNA("ACGU")
  }

  it should "validate dna" in {
    a [RuntimeException] should be thrownBy new DNA("John")
  }

  it should "count all nucleotides" in {
    val s = "AGCTTTTCATTCTGACTGCAACGGGCAATATGTCTCTGTGTGGATTAAAAAAAGAGTGTCTGATAGCAGC"
    val dna = new DNA(s)
    val expected = Map('A' -> 20, 'T' -> 21, 'G' -> 17, 'C' -> 12)
    dna.nucleotideCounts should be (expected)
  }
}

Source

Answer 1

You're right that this doesn't feel like functional-programming.

To start, avoid mutable collections; mutability goes against functional programming principles.

This problem should be approached as some kind of fold operation: you start with some initial result (a map with zero counts), then transform it for each letter encountered in the string.

You shouldn't need to validate the string as a separate step; you can validate as you go.

A type annotation of the result would be a good idea.

If your only operation on a DNA sequence is to perform a nucleotide count, I don't think you need a class. A "static" function on a DNA object, called like val counts = DNA.nucleotideCounts("AACTG"), will do.

object DNA {
  def nucleotideCounts(dnaSequence: String): Map[Char, Int] = {
    val zero = Map('A' -> 0, 'T' -> 0, 'C' -> 0, 'G' -> 0)
    dnaSequence.foldLeft(zero)((counts, nucleotide) =>
      counts.updated(nucleotide, counts.get(nucleotide) match {
        case Some(n) => n + 1
        case None    => throw new RuntimeException()
      })
    )
  }
}

Or, you could implement the original interface using the fold technique:

class DNA(dnaSequence: String) {
  private val zero = Map('A' -> 0, 'T' -> 0, 'C' -> 0, 'G' -> 0)
  private val counts = dnaSequence.foldLeft(zero){ (counts, nucleotide) =>
    counts.updated(nucleotide, counts.get(nucleotide) match {
      case Some(n) => n + 1
      case None    => throw new RuntimeException()
    })
  }

  def nucleotideCounts(): Map[Char, Int] = counts
  def nucleotideCounts(nucleotide: Char): Int = counts.get(nucleotide).get
}

Answer 2

Encapsulation and information hiding

The parameterless nucleotideCounts method exposes the map of counts. If later you wanted to change the way you store the counts, for example you wanted to store in a database instead of a map in memory, you won't be able to do that, because you did not hide this information.

Exposing the map is especially suspicious since the map is mutable. Users of this class could circumvent the validation logic, for example delete nucleotides or add invalid nucleotides.

In any case, it would be better to declare the count map private.

Unnecessary getOrElse

Thanks to the validate method, and if we assume that users of the parameterless nucleotideCounts method will not abuse the class and delete nucleotides, it's guaranteed that count.get(c).get will always find a value, so you can replace all occurrences of count.get(c).getOrElse(0).

Mutable or not

It's not clear why you use a mutable map. Your current implementation and tests would work with an immutable map too.

If you really want to use a mutable map, then you can replace this:

count = count.updated(i, count.get(i).getOrElse(0) + 1)

with:

count.update(c, count.get(c).get + 1)

Naming

count is a not a great name for a collection type. For example the plural counts would be a more fitting name, which would also match the naming of your parameterless nucleotideCounts method.

You used multiple names for a character in a DNA string: i, chr, nucleotide. It would be easier to read the code if you used a consistent name. i is especially a bad name, as it's typically used in counting loops.

Answer 3

The following code passes your tests and uses immutable data structures.

object DNA {
  val nuclSet = Set('A', 'G', 'C', 'T')                           
  val nuclMap = Map('A' -> 0, 'T' -> 0, 'C' -> 0, 'G' -> 0)
}

class DNA(sequence: String) {
  if ((Set(sequence:_*) union DNA.nuclSet) != DNA.nuclSet) {
    throw new RuntimeException()
  }

  val nucleotideCounts = {
    val m0 = sequence.toSeq.groupBy(c => c).mapValues(_.size)
    DNA.nuclMap.map { case (k, v) => k -> m0.getOrElse(k, 0) }
  }
}

The DNA object is what is known as a companion object. In this case we might use it to store values that are the same across all instantiations of the DNA class .

Within the DNA class itself we start off by checking that the Char's in sequence belong to the set of allowable nucleotides.

Finally we calculate nucleotideCounts. The crux of the calculation rests in the use of groupBy followed with a mapValues. If you are unfamiliar with these methods I'd suggest playing around with them in the REPL as they come in handy.