I found the bug when I was learning the t-w-r feature. But just as Stuart thoroughly analysis, I failed to find a easy way to use t-w-r for these two updates.
Thanks for all of the feedback. Xuelei On Jun 29, 2011, at 6:44 AM, Brad Wetmore <[email protected]> wrote: > While I was thinking the same thing as Sean/Stuart, it's not clear how much > risk you would be refactoring a lot of code just to use this language feature. > > It's pretty clear (to me anyway) what you were intending here. > > Brad > > > > > On 6/28/2011 1:31 PM, Stuart Marks wrote: >> On 6/28/11 11:46 AM, Sean Mullan wrote: >>> On 6/27/11 9:29 PM, Xuelei Fan wrote: >>>> webrev: http://cr.openjdk.java.net/~xuelei/7059709/webrev.00/ >>> >>> It seems you could restructure this code by moving the instantiation >>> of the >>> FileInputStream down just before the KeyStore.load and use a >>> try-with-resources >>> block instead. >> >> I had thought of this too but using try-with-resources for either of >> these cases isn't obvious. >> >> One difficulty is that the FileInputStreams are only initialized if some >> condition is true, otherwise they're left null. That is, simplified, >> >> FileInputStream fis = null; >> if (...condition...) { >> fis = ...initialization... >> } >> // process fis if non-null >> >> To use try-with-resources, you'd have to do something like put the >> conditional within the initialization expression. The only way to do >> this in-line in Java is to use the ternary (?:) operator, like so: >> >> try (FileInputStream fis = ...condition... ? ...initialization... : null) { >> // process fis if non-null >> } >> >> or the conditional could be evaluated prior to the try, with the >> resource variable being an alias: >> >> FileInputStream fis = null; >> if (...condition...) { >> fis = ...initialization... >> } >> try (FileInputStream fis2 = fis) { >> // process fis2 if non-null >> } >> >> or even by refactoring the conditional initialization into a separate >> method: >> >> try (FileInputStream fis = openFileOrReturnNull(...)) { >> // process fis if non-null >> } >> >> Another approach would be to call KeyStore.load() from the different >> condition arms, instead of conditionally initializing a variable: >> >> if (...condition...) { >> try (FileInputStream fis = ...initialization...) { >> ks.load(fis, passwd); >> } >> } else { >> ks.load(null, passwd); >> } >> >> This initializes the input streams closer to where they're used, but >> this might change the behavior if an error occurs while opening the >> stream; additional initializations or even side effects might occur >> before the error is reached. I haven't inspected the code thoroughly to >> determine whether this is the case. It also puts big initialization >> expression into the t-w-r, which might be ugly. >> >> Now, as you might know, I'm a big fan of try-with-resources, but using >> it here brings in a potentially large restructuring. This might or might >> not be warranted for this particular change; I don't know the tradeoffs >> involved in this code. >> >> s'marks
