CODEWITHSUNDEEP
javascalacompare
Freewind
Freewind

Reputation: 198238

How to improve this scala code which is checking if two byte arrays are same?

I have a method to compare two byte arrays. The code is java-style, and there are many "if-else"s.

def assertArray(b1: Array[Byte], b2: Array[Byte]) {
  if (b1 == null && b2 == null) return;
  else if (b1 != null && b2 != null) {
    if (b1.length != b2.length) throw new AssertionError("b1.length != b2.length")
    else {
      for (i <- b1.indices) {
        if (b1(i) != b2(i)) throw new AssertionError("b1(%d) != b2(%d)".format(i, i))
      }
    }
  } else {
    throw new AssertionError("b1 is null while b2 is not, vice versa")
  }
}

I have tried as following, but it's not simplified the code much:

(Option(b1), Option(b2)) match {
    case (Some(b1), Some(b2)) => if ( b1.length == b2.length ) {
       for (i <- b1.indices) {
        if (b1(i) != b2(i)) throw new AssertionError("b1(%d) != b2(%d)".format(i, i))
       }
    } else {
       throw new AssertionError("b1.length != b2.length")
    }
    case (None, None) => _
    case _ => throw new AssertionError("b1 is null while b2 is not, vice versa")
}

Upvotes: 2

Views: 4164

Answers (5)

爱国者
爱国者

Reputation: 4338

Here is my solution using tail recursive:

@scala.annotation.tailrec
def assertArray[T](b1: Array[T], b2: Array[T])(implicit m: Manifest[T]) : Unit = (b1, b2)  match{
    case (null, null) => 
    case (null, a) if a != null => throw new AssertionError 
    case (a, null) if a != null => throw new AssertionError  
    case (Array(), Array()) => 
    case _  => if (b1.length != b2.length ||  b1.head != b2.head ) throw new AssertionError  else assertArray(b1.tail,b2.tail)  
}

and the test casees

assertArray(null,null)
assertArray(Array[Byte](),null)
assertArray(null,Array[Byte]())
assertArray(Array[Byte](),Array[Byte]())
assertArray(Array[Byte](),Array[Byte](1))
assertArray(Array[Byte](1,2,3),Array[Byte](1,2,3))
assertArray(Array[Byte](1,3),Array[Byte](1))

How about this https://gist.github.com/1322299 link

Upvotes: 1

user unknown
user unknown

Reputation: 36229

A minor improvement to Matthew's solution could be, to return all diffs and not just the first:

def assertArray (b1: Array[Byte], b2: Array[Byte]): Unit = {

  def diffs [T] (a: Array[T], b: Array[T]) = 
    (a.zip (b).filter (e => (e._1 != e._2)))

  if (b1 == null && b2 == null) 
    return;
  if (b1 == null || b2 == null) 
    throw new AssertionError ("b1 is null while b2 is not, vice versa")  
  if (b1.length != b2.length) 
    throw new AssertionError ("b1.length != b2.length")
  val delta = diffs (b1, b2)
  delta.map (d => throw new AssertionError ("" + delta.mkString ))
}

Test invocation:

val ab = (List ((List (47, 99, 13, 23, 42).map (_.toByte)).toArray,
  (List (47, 99, 33, 13, 42).map (_.toByte)).toArray)).toArray

assertArray (ab(0), ab(1))
// java.lang.AssertionError: (13,33)(23,13)

Upvotes: 1

Matthew Farwell
Matthew Farwell

Reputation: 61705

Unless you're doing this as an academic exercise, how about

java.util.Arrays.equals(b1, b2)

The description:

Returns true if the two specified arrays of bytes are equal to one another. Two arrays are considered equal if both arrays contain the same number of elements, and all corresponding pairs of elements in the two arrays are equal. In other words, two arrays are equal if they contain the same elements in the same order. Also, two array references are considered equal if both are null.

I will admit to this being 'java style' :-)

Since you're throwing AssertionErrors, you can remove all of the else's:

def assertArray(b1: Array[Byte], b2: Array[Byte]): Unit = {
  if (b1 == b2) return;

  if (b1 == null || b2 == null) throw new AssertionError("b1 is null while b2 is not, vice versa")  

  if (b1.length != b2.length) throw new AssertionError("b1.length != b2.length")

  for (i <- b1.indices) {
    if (b1(i) != b2(i)) throw new AssertionError("b1(%d) != b2(%d)".format(i, i))
  }
}

If, as I suspect, you're actually using this within JUnit tests (hence the assertArray), then you can use a trick which I often do, compare the string representations of the arrays:

def assertArray2(b1: Array[Byte], b2: Array[Byte]): Unit = {
  assertEquals(toString(b1), toString(b2))
}

def toString(b: Array[Byte]) = if (b == null) "null" else java.util.Arrays.asList(b:_*).toString

which will give you the same outcome (an AssertionError), with where the differences are.

Upvotes: 12

Paul Butcher
Paul Butcher

Reputation: 10852

The standard library provides sameElements for exactly this purpose:

scala> val a1 = Array[Byte](1, 3, 5, 7); val a2 = Array[Byte](1, 3, 5, 7); val a3 = Array[Byte](1, 3, 5, 7, 9)
a1: Array[Byte] = Array(1, 3, 5, 7)
a2: Array[Byte] = Array(1, 3, 5, 7)
a3: Array[Byte] = Array(1, 3, 5, 7, 9)

scala> a1 sameElements a2
res0: Boolean = true

scala> a1 sameElements a3
res1: Boolean = false

Upvotes: 6

tenshi
tenshi

Reputation: 26566

One possible simplification:

def assertArray(b1: Array[Byte], b2: Array[Byte]) {
    (Option(b1), Option(b2)) match {
        case (None, _) => 
            throw new AssertionError("b1 is null")
        case (_, None) => 
            throw new AssertionError("b2 is null")
        case (Some(Size(b1Size)), Some(Size(b2Size))) if b1Size != b2Size  => 
            throw new AssertionError("b1.length != b2.length")
        case (Some(b1), Some(b2)) if b1 zip b2 find (c => c._1 != c._2) isDefined => 
            throw new AssertionError("Arrays do not match")
        case _ => // everything is OK
    }
}

object Size {
    def unapply[T](arr: Array[T]): Option[Int] = Some(arr.size)
}

Probably can be improved even more, but at least it does not have nested ifs and external loops.

Upvotes: 1

Related Questions