|
|
On Mon, Jan 17, 2011 at 7:37 PM, John Ky <[hidden email]> wrote:
Hi Jim,
That's an interesting concept, however, doesn't it mean that if a fails then b, c, and d will still be created only to be discarded?
Yes, but is this a valid objection or is it premature optimization? In your version, a, a and b, or a and b and c will be created only to be discarded if the later allocations fail. And if an earlier allocation fails because of a lack of resources or other persistent error, then the later ones will too and so they won't be created only to be discarded. If Resource.open takes parameters and a, b, c, and d are independent, then you might well want to know their failure conditions independently -- e.g., if I try to concatenate four files, I would consider it a feature to report *all* of those that can't be opened and why, rather than reporting on only the first failure. And if a, b, c, and d aren't independent, then presumably the later allocations will fail because the earlier ones did.
But if you really really need to optimize the failure cases, Resource.open could take a Resource* that is checked before allocation, returning failure if any of them were failures: class Foo {
// each is only allocated if all of the preceding succeeded
val a = new Resource.open()
val b = new Resource.open(a,b)
val c = new Resource.open(a,b,c)
val d = new Resource.open(a,b,c,d)
if( d.failed ) Resource.closeAndThrow(a,b,c,d) // among other possibilities
}
Also Resource.needAll is called from the primary constructor. However the primary constructor can't be used to return values. Doesn't it mean Resource.needAll must communicate errors by throwing an exception and as a result is contrary to the practise of returning error values?
i.e. I can't then compose Foo into Goo:
class Goo { val a = new Foo()
val b = new Foo() val c = new Foo()
val d = new Foo() Foo.needAll(a, b, c, d)
}
because Foo doesn't follow the suggested coding style.
If you want a return value, use a factory method, as you did with Resource.open. A private constructor would throw the exception, which would be packaged into a failure instance by the factory method. If you call Foo's factory method "apply", you can simply omit the "new"s above.
-- Jim On 18 January 2011 13:32, Jim Balter <[hidden email]> wrote:
Unfortunately, your code creates a couple of useless private tuple fields to hold the temporary.
But ... this doesn't seem like a very good approach to your use case. You might consider having a subclass of Resource that represents a failure to allocate a resource, including info about why -- it might even just be a wrapper for an Exception resulting from attempting to open the resource. Then you can just check to see if any of a,b,c,d contains such a failure. One possibility is Resource.needAll(rs: Resource*) that checks to see if any of its arguments represents a failure and if so closes the others and then throws an Exception (the thrown Exception should do the appropriate method overrides to combine the info of the failing cases); then you could simply write
class Foo {
val a = Resource.open()
val b = Resource.open() val c = Resource.open()
val d = Resource.open() Resource.needAll(a, b, c, d) }
There are numerous other straightforward possibilities once you abandon the control mess created by throwing Exceptions and return failure instances instead.
-- Jim
Hi Paul,
That sounds quite alright.
However, the use case I was struggling with is something like this:
class Foo { val a = Resource.open() val b = Resource.open()
val c = Resource.open() val d = Resource.open()
}
If an exception is thrown anywhere in the constructor, I need to close any resources already opened, so something like this:
class Foo { val (a, b, c, d) = {
val tempA = closer.closes(Resource.open()) try {
val tempB = closer.closes(Resource.open()) try {
val tempC = closer.closes(Resource.open()) try {
val tempD = closer.closes(Resource.open()) (tempA, tempB, tempC, tempD)
} catch { case e: Exception =>
tempC.close() throw e
} } catch { case e: Exception =>
tempB.close() throw e
} } catch { case e: Exception =>
tempA.close() throw e
} }
}
Or if I write a Closer class, which represents a list of objects to close, I can reduce it to this:
class Foo { val closer = new Closer()
val (a, b, c, d) = try { val tempA = closer.closes(Resource.open())
val tempB = closer.closes(Resource.open()) val tempC = closer.closes(Resource.open())
val tempD = closer.closes(Resource.open()) (tempA, tempB, tempC, tempD)
} catch { case e: Exception => closer.closeAll()
throw e } }
Can I do any better?
Cheers,
-JohnOn 18 January 2011 11:55, Paul Phillips <[hidden email]> wrote:
On Tue, Jan 18, 2011 at 11:35:58AM +1100, John Ky wrote:
> That works. How about if I have lots of fields and want to share the
> same catch clause for all of them?
The present possibilities are fairly unfortunate, but I have some good
stuff in 2.9. Write a fancy catch block once (or a fancy catch block
creating function) and reuse it. Off the cuff:
object Test {
def handler[T](value: T): PartialFunction[Throwable, T] = {
case x: NullPointerException => println("NPE") ; value
case x: Exception => println(x) ; value
}
// drink in the beauty of the braces which do not exist
val x = (
try sys.error("oops")
catch handler(5)
finally println("fin")
)
val y = try null.toString catch handler("NPE")
def main(args: Array[String]): Unit = { }
}
// output
% scala29 Test
java.lang.RuntimeException: oops
fin
NPE
--
Paul Phillips | It's better to have gloved and tossed than never to
Protagonist | have played baseball.
Empiricist |
i pull his palp! |----------* http://www.improving.org/paulp/ *----------
|
Locked
