current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

I'm trying to get my head in the Scala game, and would welcome feedback on the following code, which produces a table from data.

class TabulatorTest extends FunSuite with ShouldMatchers {

  test("format") {
    Tabulator.format(List(
      List("head1", "head2", "head3"),
      List("one", "two", "three"), 
      List("four", "five", "six"))) should be ("""
+-----+-----+-----+
|head1|head2|head3|
+-----+-----+-----+
|  one|  two|three|
| four| five|  six|
+-----+-----+-----+
""".trim)
  }

  test("format empty") {
    Tabulator.format(List()) should be ("")
    Tabulator.format(List(List())) should be ("""
++
||
++
++
""".trim)
  }

  test("uneven rows") {
    try {
      Tabulator.format(List(    
        List("head1", "head2", "head3"),
        List("one", "two")))
      fail()
    } catch {
      case e: IllegalArgumentException => 
    }
  }
}

object Tabulator {

  def format(table: Seq[Seq[Any]]) = table match {
    case Seq() => ""
    case _ => 
      val cellSizes = for (row <- table) yield 
        (for (cell <- row) yield 
          if (cell == null) 0 else cell.toString.length)
      val colSizes = for (col <- cellSizes.transpose) yield col.max
      val rows = for (row <- table) yield formatRow(row, colSizes)
      formatRows(rowSeparator(colSizes), rows)
  }

  def formatRow(row: Seq[Any], colSizes: Seq[Int]) = {
    val cells = (for ((item, size) <- row.zip(colSizes)) yield 
      if (size == 0) "" else ("%" + size + "s").format(item))
    cells.mkString("|", "|", "|")
  }

  def formatRows(rowSeparator: String, rows: Seq[String]): String = (
    rowSeparator :: 
    rows.head :: 
    rowSeparator :: 
    rows.tail.toList ::: 
    rowSeparator :: 
    List()).mkString("\n")


  private def rowSeparator(colSizes: Seq[Int]) =
    colSizes map { "-" * _ } mkString("+", "+", "+")
}
share|improve this question
up vote 3 down vote accepted

I can't help a lot. Only some syntactic hints:

object Tabulator {

  def format(table: Seq[Seq[Any]]) =
    if (table.isEmpty) ""
    else {
      def cellSize(cell: Any) =
        cell.toString.length

      val cellSizes =
        table map { _ map cellSize }
      val colSizes =
        cellSizes.transpose map { _.max }
      val rows =
        table map formatRow(colSizes)

      formatRows(rowSeparator(colSizes), rows)
    }

  def formatRow(colSizes: Seq[Int])(row: Seq[Any]) = {
    val cells =
      for ((item, size) <- row zip colSizes) yield
        if (size == 0) "" else "%"+size+"s" format item
    cells mkString ("|", "|", "|")
  }

  def formatRows(rowSeparator: String, rows: Seq[String]): String = (
       rowSeparator
    :: rows.head
    :: rowSeparator
    :: rows.tail.toList
    ::: rowSeparator
    :: Nil
  ) mkString "\n"


  private def rowSeparator(colSizes: Seq[Int]) =
    colSizes map { "-"*_ } mkString ("+", "+", "+")
}

I deleted the null-check in cellSize because null is not familiar in Scala. If you definitely need null then restore the check.

share|improve this answer
    
This is not null safe. You shoudl fix cellSize to use Option(cell) map (_.toString.length) getOrElse 0. – Daniel C. Sobral Oct 3 '11 at 21:21
    
@DanielC.Sobral: I know that it is not null safe. But in Scala typically nobody uses null. And because I don't know what data the code has to handle I deleted the null-check. There can be null but maybe null is handled before the Tabulator is used? To your second comment: There is formatRow and formatRow s – sschaef Oct 3 '11 at 21:34
    
Should I favour if over case for simple tests? I rather like the shape of the case rather than the annoying brackets and braces in if statements, and the case gives a nicer sense of different function bodies for different arguments like Lisp and Haskell. – Duncan McGregor Oct 3 '11 at 21:38
    
@Antoras - it turns out that the Java objects that this is used to display often have null fields. – Duncan McGregor Oct 3 '11 at 21:40
    
@Antoras - why define cellSize as a nested function, but have others as methods? Aren't I just loosing a place I could customise through subclassing? – Duncan McGregor Oct 3 '11 at 21:42

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.