HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

String Calculator Kata in Scala

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scalakatacalculatorstring

Problem

This is my solution of string calulator kata in scala(i'm new of tdd).
I'd like a general review of this.


String Calculator


Create a simple String calculator with a method int Add(string
numbers) The method can take 0, 1 or 2 numbers, and will return their
sum (for an empty string it will return 0) for example “” or “1” or
“1,2”


-Allow the Add method to handle an unknown amount of numbers


-Allow the Add method to handle new lines between numbers (instead of commas). the following input is ok: “1\n2,3” (will equal 6) the
following input is NOT ok: “1,\n” (not need to prove it - just
clarifying)


-Support different delimiters to change a delimiter, the beginning of the string will contain a separate line that looks like this:

“//[delimiter]\n[numbers…]” for example “//;\n1;2” should return three
where the default delimiter is ‘;’ .


-Numbers bigger than 1000 should be ignored, so adding 2 + 1001 = 2


-Delimiters can be of any length with the following format: “//[delimiter]\n” for example: “//[]\n12***3” should return 6

```
package stringcalculator

import org.scalatest.{BeforeAndAfter, FunSuite}

class StringCalculetor3 extends FunSuite with BeforeAndAfter {

private val calculator = new Calculator(new InputParser())

before {}

test("sum of empty string is 0") {

assert(0 === sum(""))

}

test("sum of one number string is number") {

assert(5 === sum("5"))
assert(6 === sum("6"))

}

test("sum of two numbers separated with comma") {

assert(3 === sum("1,2"))
assert(5 === sum("1,4"))

}

test("sum of 3 numbers separated with comma") {

assert(6 === sum("1,2,3"))

}

test("sum of 2 numbers separated new line separetor") {

assert(3 === sum("1\n2"))
assert(6 === sum("1\n2,3"))

}

test("sum of 2 numbers with another separator") {

assert(3 === sum("//;\n1;2"))

}

test("sum of 2 n

Solution

It is nice that you have divided the parsing and the actual calculation over the InputParser and the Calculator.

I think your InputParser can be improved at some points :

  • Using a regular expression to extract the separator would reduce the size of your InputParser and the large number of methods needed to get the separator.



  • I would also include the splitting of the input in the parser, so you can return only the numbers. The user of the parser does not really need to know which separator was used.



  • toInt throws an exception if the string can not be converted to an Int, which you don't handle. More ideomatic Scala would be to return an Option, Try, ...



  • If you add more functionality to the InputParser, you probably should also create unit tests for the InputParser class.



My attempt to improve your InputParser :

class Parser {
  private val defaultSeparator: String = ",|\n"

  // parse now returns an Option[List[Int]]
  // -> invalid input now returns None (eg parse("foo"))
  def parse(input: String): Option[List[Int]] = {
    input match {
      case "" => Some(List(0))
      case SeparatorNumbers(sep, numbers) => 
        val separator = sep.map(escapeSeparator).getOrElse(defaultSeparator)
        val stringNumbers = numbers.split(separator).toList
        // stringNumbers.map(...) is of type List[Option[Int]]
        // the sequence function below turns this into Option[List[Int]]
        // (you could also use Scalaz for sequence (or traverse), but 
        // that is probably overkill if you only use it here)
        sequence(stringNumbers.map(i => Try(i.toInt).toOption))
      case _ => None
    }
  }

  // not explicitly mentioned in the requirements
  // make multiple separators possible (eg "//[:|;|,]\n1:2;3.4")
  private def escapeSeparator(sep: String) = 
    sep.split("""\|""").map(java.util.regex.Pattern.quote).mkString("|")

  // an extractor object which uses a regular expression
  // pattern matching with SeparatorNumbers gives an optional separator
  // and the numbers (still as one string)
  object SeparatorNumbers {
    // regular expression with two capture groups for separator and the rest
    private val Extract = """(?s)^(?://\[?(.+)\]?\n)?(.*)""".r

    def unapply(input: String): Option[(Option[String], String)] = 
      input match {
        // if no separator is found, sep is null
        // Option(sep) will then be None 
        case Extract(sep, numbers) => Some(Option(sep), numbers)
        case _ => None
      }
  }
}

// possible implementation to sequence List[Option[A]]
def sequence[A](listO: List[Option[A]]) : Option[List[A]] =
  listO.foldLeft(Option(List.empty[A])) { 
    case (None, _) => None
    case (_, None) => None
    case (Some(list), Some(elem)) => Some(elem :: list)
  }.map(_.reverse)


Your Calculator class could pretty mutch stay the same, you only need to handle Option[List[Int]] instead of List[Int].

val p = new Parser

def add(input: String) = {
  p.parse(input).map(_.filter(_ < 1001).sum) 
  // now Option[Int], depends how you want to handle invalid input
}

Code Snippets

class Parser {
  private val defaultSeparator: String = ",|\n"

  // parse now returns an Option[List[Int]]
  // -> invalid input now returns None (eg parse("foo"))
  def parse(input: String): Option[List[Int]] = {
    input match {
      case "" => Some(List(0))
      case SeparatorNumbers(sep, numbers) => 
        val separator = sep.map(escapeSeparator).getOrElse(defaultSeparator)
        val stringNumbers = numbers.split(separator).toList
        // stringNumbers.map(...) is of type List[Option[Int]]
        // the sequence function below turns this into Option[List[Int]]
        // (you could also use Scalaz for sequence (or traverse), but 
        // that is probably overkill if you only use it here)
        sequence(stringNumbers.map(i => Try(i.toInt).toOption))
      case _ => None
    }
  }

  // not explicitly mentioned in the requirements
  // make multiple separators possible (eg "//[:|;|,]\n1:2;3.4")
  private def escapeSeparator(sep: String) = 
    sep.split("""\|""").map(java.util.regex.Pattern.quote).mkString("|")

  // an extractor object which uses a regular expression
  // pattern matching with SeparatorNumbers gives an optional separator
  // and the numbers (still as one string)
  object SeparatorNumbers {
    // regular expression with two capture groups for separator and the rest
    private val Extract = """(?s)^(?://\[?(.+)\]?\n)?(.*)""".r

    def unapply(input: String): Option[(Option[String], String)] = 
      input match {
        // if no separator is found, sep is null
        // Option(sep) will then be None 
        case Extract(sep, numbers) => Some(Option(sep), numbers)
        case _ => None
      }
  }
}

// possible implementation to sequence List[Option[A]]
def sequence[A](listO: List[Option[A]]) : Option[List[A]] =
  listO.foldLeft(Option(List.empty[A])) { 
    case (None, _) => None
    case (_, None) => None
    case (Some(list), Some(elem)) => Some(elem :: list)
  }.map(_.reverse)
val p = new Parser

def add(input: String) = {
  p.parse(input).map(_.filter(_ < 1001).sum) 
  // now Option[Int], depends how you want to handle invalid input
}

Context

StackExchange Code Review Q#108891, answer score: 6

Revisions (0)

No revisions yet.