Part of the problem is all the exceptions. There are better ways of handling exceptions, such as Scalaz Validation or Lift's Box. Scala itself comes with Either
, which isn't particularly flexible.
On the other hand, you are not returning anything, which actually turns the whole code into a reverse Option
: you either have Some(exception)
or None
.
Now, the test itself, except for checking for nulls, has a name in Scala: sameElements
. Only it will not tell you what the problem was.
I can think of two ways of handling it. The first is just a small improvement on your pattern matching:
def assertArray(b1: Array[Byte], b2: Array[Byte]) = (Option(b1), Option(b2)) match {
case (None, None) =>
case (None, _) | (_, None) => throw new AssertionError("b1 is null while b2 is not, vice versa")
case (Some(b1), Some(b2)) if b1.length == b2.length =>
(b1, b2).zipped.map(_ == _)
.zipWithIndex.find(!_._1)
.foreach {
case (_, i) => throw new AssertionError("b1(%d) != b2(%d)".format(i, i))
}
case _ => throw new AssertionError("b1.length != b2.length")
}
The other way would be to turn the problem around completely. It would be like this:
def assertArray(b1: Array[Byte], b2: Array[Byte]) = {
def isOneNull =
if ((b1 eq null) ^ (b2 eq null)) Some("b1 is null while b2 is not, vice versa")
else None
def areSizesDifferent =
if (b1.length != b2.length) Some("b1.length != b2.length")
else None
def haveDifferentElements =
(b1, b2).zipped.map(_ == _)
.zipWithIndex.find(!_._1)
.map { case (_, i) => "b1(%d) != b2(%d)" format (i, i) }
def areTheyDifferent =
if ((b1 ne null) && (b2 ne null)) areSizesDifferent orElse haveDifferentElements
else isOneNull
areTheyDifferent foreach (msg => throw new AssertionError(msg))
}
This is longer, and handles nullness in two separate places, and doesn't protect against nullness, but I think it reads much better. To get more than this I need Scalaz:
def assertArray(b1: Array[Byte], b2: Array[Byte]) = {
def isOneNull =
(b1 eq null) ^ (b2 eq null) option "b1 is null while b2 is not, vice versa"
def areTheyDifferent =
Option(b1) <|*|> Option(b2) flatMap {
case (b1, b2) =>
def areSizesDifferent = b1.length != b2.length option "b1.length != b2.length"
def haveDifferentElements =
(b1, b2).zipped.map(_ == _)
.zipWithIndex.find(!_._1)
.map { case (_, i) => "b1(%d) != b2(%d)" format (i, i) }
areSizesDifferent orElse haveDifferentElements
} orElse isOneNull
areTheyDifferent foreach (msg => throw new AssertionError(msg))
}
Or, to go more full-blown with Scalaz:
def assertArray(b1: Array[Byte], b2: Array[Byte]) = {
type Params = (Array[Byte], Array[Byte])
type Func = Params => Option[String]
def isOneNull =
(b1 eq null) ^ (b2 eq null) option "b1 is null while b2 is not, vice versa"
def areSizesDifferent: Func = {
case (b1, b2) => b1.length != b2.length option "b1.length != b2.length"
}
def haveDifferentElements: Func = {
case (b1, b2) =>
(b1, b2).zipped.map(_ == _)
.zipWithIndex.find(!_._1)
.map { case (_, i) => "b1(%d) != b2(%d)" format (i, i) }
}
def areTheyDifferent =
Option(b1) <|*|> Option(b2) flatMap (
areSizesDifferent |@| haveDifferentElements
apply (_ orElse _)
) orElse isOneNull
areTheyDifferent foreach (msg => throw new AssertionError(msg))
}
Scala has an unfortunate overhead compared to Haskell to do these things. Also, Scalaz will be able to do a bit more in the next version, but this works with 6.0.
The gain with Scalaz is not, however, legibility or conciseness (in this code, anyway), but of composition. For instance, in the current Scalaz we can abstract most of the body of areTheyDifferent
like this:
def composeFM[M[_]:Plus, F[_]:Applicative, B](f: F[M[B]], g: F[M[B]]): F[M[B]] =
(f |@| g)(_ <+> _)
type FAB[x] = Array[Byte] => x
def areTheyDifferent = (
Option(b1) <|*|> Option(b2)
flatMap composeFM[Option, FAB, Int](areSizesDifferent, haveDifferentElements)
orElse isOneNull
)
Note that composeFM
doesn't know a thing about Option
or Function1
-- it applies to the pattern.
Well, anyway, take your pick. Sometimes a problem just isn't worth the trouble, but it is useful to know how to handle the trouble anyway.