9
\$\begingroup\$

I'm quite new to Scala and was surprised that there doesn't seem to be anything like Java's try-with-resources block. So of course I wrote my own little construct. Is there anything I could improve?

import scala.util.control.NonFatal
import scala.util.Try
import scala.util.Failure
import scala.util.Success

object TryWith {
  def apply [Closable <: { def close() }, Result]
      (resGen: => T) (r: (Closable) => Result): Try[Result] = {

    Try(resGen) match {
      case Success(closable) => {
        try Success(r(closable)) catch {
          case NonFatal(e) => Failure(e)
        } finally {
          try {
            closable.close()
          } catch {
            case e: Exception => {
              System.err.println("Failed to close Resource:")
              e.printStackTrace()
            }
          }
        }
      }
      case Failure(e) => Failure(e)
    }
  }
}

Examples:

val res = TryWith(Source.fromFile("foo.bar")) {
  _.mkString
}

res match {
  case Success(str) => println(str)
  case Failure(e) => e.printStackTrace()
}

--

TryWith(new BufferedReader(new FileReader("foo.bar")))((reader)=> {
  var line = reader.readLine()
  while(line != null) {
    println(line)
    line = reader.readLine()
  } 
})

Right now there is no way to react to or even detect a failed attempt to close the resource. I can see four ways to make that possbile but I'm not that happy with any of them:

  1. Instead of catching the Exception, let it through. It can now be dealt with but kind of defeats the purpose of the whole TryWith block.

  2. Return a Failure[Result] if closing fails. I don't like this because the action was actually a success and there is a valid result. By returning a Failure that result is lost.

  3. Return a pair of Trys, i.e. (Try[Result], Try[Unit]). The first one is the result of the operation, the second one is the result of the closing.

  4. Pass a (Closable) => Unit error handler as argument. This would also allow trying to close the resource again (which most likely won't change anything though).

Is handling a failed close even important enough to think about? I know it can happen but usually there is no way to recover from it.

\$\endgroup\$
14
\$\begingroup\$

There are a couple of places where you can improve.

First the pedantic stuff.

  • Closable should probably be Closeable
  • Cleaning up your imports can make it easier to get to the parts of your program that matter. In this case you can consolidate your imports to a much simpler version:

    import scala.util.control.NonFatal
    import scala.util.{Failure, Success, Try}
    
  • Make sure it compiles before posting. In this case, there was a missed rename, and it needed an additional import to compile without warnings (needed by the reflected call to #close on a structurally typed value).

  • Several of your pairs of curly braces are optional, and removing them makes it much easier to read.

  • Scala convention favors single character for type parameters. I'm not sure that I 100% agree with this particular convention for classes with type parameters, but for a single method it's best to stick to the convention.

Ok, now that's out of the way, to the good stuff :)

Use interfaces if they are already there

Scala's interoperability with Java is one of it's strengths, so re-inventing an interface isn't really needed. Additionally, using java.io.Closeable gets rid of the structural typing, which is a nifty feature, but it's better to avoid if it's not strictly needed. This simplifies things considerably, particularly the type signature.

FlatMap

Try is a Monad, which means a bunch of stuff to category theory buffs - which I don't particularly care about. What I do care about Monads is that they have two methods defined on them that can make life much easier. flatMap is the first, and you've basically defined the content of this method in your outermost pattern match.

Refactored Version 1

import java.io.Closeable
import scala.util.control.NonFatal
import scala.util.{Failure, Success, Try}

object TryWith {
  def apply [C <: Closeable, R](resGen: => C) (r: Closeable => R): Try[R] =
    Try(resGen).flatMap(closeable => {
      try {
        Success(r(closeable))
      }
      catch {
        case NonFatal(e) => Failure(e)
      }
      finally {
        try {
          closeable.close()
        }
        catch {
          case e: Exception =>
            System.err.println("Failed to close Resource:")
            e.printStackTrace()
        }
      }
    })
}

Principle of Least Surprise

So far we haven't changed how the method actually works. In the possibilities you mentioned for how to handle a failure to close you missed the most important option: mimic the behavior of the Java try-with-resources block.

This is important for two reasons: the first is that it's a pretty sane way of handling this case, the second is that it will be the behavior expected by anyone coming from a Java background. Acting in an intuitive manner is a very powerful thing.

Java handles failures to close in this manner:

  1. If the block throws an exception, give priority to this exception. The exception thrown by #close is added as a suppressed exception, and the modified exception is thrown.
  2. If the block did not throw an exception, allow the exception from close to propagate upward.

The basic premise is that exceptions are exceptional, so they should never be swallowed.

Refactored Version 2

import java.io.Closeable
import scala.util.control.NonFatal
import scala.util.{Failure, Try}

object TryWith {
  def apply[C <: Closeable, R](resource: => C)(f: C => R): Try[R] =
    Try(resource).flatMap(resourceInstance => {
      try {
        val returnValue = f(resourceInstance)
        Try(resourceInstance.close()).map(_ => returnValue)
      }
      catch {
        case NonFatal(exceptionInFunction) =>
          try {
            resourceInstance.close()
            Failure(exceptionInFunction)
          }
          catch {
            case NonFatal(exceptionInClose) =>
              exceptionInFunction.addSuppressed(exceptionInClose)
              Failure(exceptionInFunction)
          }
      }
    })
}

The Test Suite

import java.io.Closeable
import org.scalatest.{ Matchers, WordSpec }
import scala.util.{ Failure, Success }

class TryWithSpec extends WordSpec with Matchers {
  // Exceptions and errors here so we don't pay the stack trace creation cost multiple times
  val getResourceException = new RuntimeException
  val inFunctionException = new RuntimeException
  val inCloseException = new RuntimeException
  val getResourceError = new OutOfMemoryError
  val inFunctionError = new OutOfMemoryError
  val inCloseError = new OutOfMemoryError

  val goodResource = new Closeable {
    override def toString: String = "good resource"
    def close(): Unit = {}
  }

  "TryWith" should {
    "catch exceptions getting the resource" in {
      TryWith(throw getResourceException)(println) shouldBe Failure(getResourceException)
    }

    "catch exceptions in the function" in {
      TryWith(goodResource){
        _ => throw inFunctionException
      } shouldBe Failure(inFunctionException)
    }

    "catch exceptions while closing" in {
      TryWith(new Closeable {
        def close(): Unit = throw inCloseException
      })(_.toString) shouldBe Failure(inCloseException)
    }

    "note suppressed exceptions" in {
      val ex = new RuntimeException
      val result = TryWith(new Closeable {
        def close(): Unit = throw inCloseException
      })(_ => throw ex)

      result shouldBe Failure(ex)
      val Failure(returnedException) = result
      returnedException.getSuppressed shouldBe Array(inCloseException)
    }

    "propagate errors getting the resource" in {
      intercept[OutOfMemoryError] {
        TryWith(throw getResourceError)(println)
      } shouldBe getResourceError
    }

    "propagate errors in the function" in {
      intercept[OutOfMemoryError] {
        TryWith(goodResource){
          _ => throw inFunctionError
        }
      } shouldBe inFunctionError
    }

    "propagate errors while closing" in {
      intercept[OutOfMemoryError] {
        TryWith(new Closeable {
          def close(): Unit = throw inCloseError
        })(_.toString)
      } shouldBe inCloseError
    }

    "return the value from a successful run" in {
      TryWith(goodResource)(_.toString) shouldBe Success("good resource")
    }
  }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ It's a wonderful read \$\endgroup\$ – Vivek Ragunathan Apr 27 '18 at 21:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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