current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

My implementation works, but it looks very ugly. Curious to see how it's better implemented.

The zipWith function takes a list of lists (List[List[A]]), or a row with columns if you like. It will then transform this into a column with rows and applies a function List[A]=>B to obtain a List[B]. Hopefully the unit test below explains this better.

  test("zipWith"){
    val input = List(List(1,2,3), List(4,5,6), List(7,8,9))
    val expected = List("1::4::7", "2::5::8", "3::6::9")
    assert(zipWith(input)(_.mkString("::")) === expected)
  }

and the implementation is:

  def zipWith[A, B](lists: List[List[A]])(f: List[A] => B): List[B] = {
    @tailrec
    def loop(acc: List[B], input: List[List[A]]): List[B] = {
      val init = (Nil: List[List[A]], Nil: List[A])
      val (left, zipped) = input.foldLeft(init)((z, a) => {
        val (tails, heads) = z
        (a.tail :: tails, a.head :: heads)
      })
      if (left.foldLeft(false)(_ || _.isEmpty))
        f(zipped.reverse) :: acc
      else
        loop(f(zipped.reverse) :: acc, left.reverse)
    }

    loop(Nil, lists).reverse
  }
share|improve this question
    
Can you explain us more about what you are exactly trying to accomplish with your code? Please edit it into the question. –  skiwi Nov 28 '14 at 10:44
    
@skiwi: Added some explanation. Hopefully the test will make it clear.. –  BasilTomato Nov 28 '14 at 11:04
1  
You do realize that the code can be replaced with this and will achieve the same thing: def zipWith[A, B](lists: List[List[A]])(f: List[A] => B): List[B] = lists.map(f)? Are you looking at improving your style? –  Akos Krivachy Nov 28 '14 at 19:18
    
@AkosKrivachy post an answer with that! –  janos♦ Nov 28 '14 at 20:13
    
@AkosKrivachy: No that will result in List(1::2::3, 4::5::6, 7::8::9) –  BasilTomato Nov 29 '14 at 0:31

2 Answers 2

up vote 3 down vote accepted

How about this?

def zipWith[A, B](ts: Traversable[Traversable[A]])(f: Traversable[A] => B): Traversable[B] = {
  ts.transpose.map(f)
}

Example:

scala> zipWith(input)(_.mkString("::"))
res10: Traversable[String] = List(1::4::7, 2::5::8, 3::6::9)
share|improve this answer
up vote 1 down vote

Two things:

  • Your variable names could be better, I would rather use rows and columns as a reader can understand it easier.
  • List is an implementation of Seq, you should probably use Seq where you can.

With that said I would do something like this:

def zipWith[A, B](lists: Seq[Seq[A]])(f: Seq[A] => B): Seq[B] = {
  lists.headOption.map(_.size) match {
    case None =>
      // Input list is empty, return empty
      Seq.empty[B]
    case Some(columnCount) =>
      // Do a sanity check, based on column length of first row.
      require(lists.forall(_.size == columnCount), "Can't zip rows that have differing column lengths.")
      val initial = Seq.fill(columnCount)(Seq.empty[A])
      val zippedByColumns = lists.foldLeft(initial) {
        case (columns, nextRow) =>
          columns.zip(nextRow).map{ case (column, rowElement) => column :+ rowElement }
      }
      zippedByColumns.map(f)
  }
}
share|improve this answer
1  
Nope that will result in List(1::2::3, 4::5::6, 7::8::9). It needs to be List("1::4::7", "2::5::8", "3::6::9") –  BasilTomato Nov 29 '14 at 0:31
    
@BasilTomato Thank you for pointing that out. Updated my answer with an actual solution. –  Akos Krivachy Nov 29 '14 at 1:10
    
I've seen it stated that one should prefer Vector to List as a concrete instantiation of Seq (but obviously use Seq as arguments where possible) as Vector ends up being better at things List should be better at anyways –  geoffjentry Dec 2 '14 at 1:36
    
Why not match on the headOption and then use the .size on the Some? –  geoffjentry Dec 2 '14 at 1:38

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.