Skip to main content

Please review my fix for CR 6495275

10 replies [Last post]
ersh
Offline
Joined: 2006-10-18

The CR describes usability problem -
Logviewer - save log - enter wrong file name such as "/fdfdsdf/fdsfdfsdf/fdsfsdf" for unix, click save -
there is an exception on the console.

Changes are here -
https://jtharness.dev.java.net/source/browse/jtharness?view=rev&rev=1536

Thanks, Mike

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
ersh
Offline
Joined: 2006-10-18

Thank you, Brian!

fda
Offline
Joined: 2005-05-27

Would it make sense to show the reason of error like an exception message, not only the error itself:
uif.showError("logviewer.cantwritereport");

ersh
Offline
Joined: 2006-10-18
fda
Offline
Joined: 2005-05-27

fine by me

bkurotsu
Offline
Joined: 2004-12-13

It's ok this way. But since we are here making it perfect, maybe the full exception should be logged in the FileNotFound catch.

And about catching other exceptions, yes, the is an outer catch try-catch block - why doesn't it behave similar to the inner one and put up a dialog? Maybe both blocks should clear the wait dialog, show the error dialog and send details to the logging system.

ersh
Offline
Joined: 2006-10-18

Clearing the wait dialog in the others catch blocks is a nice catch, thanks!
I moved this to the finally block of enclosing try

As for logging... I don't like this because user will see two similar error messages in the same time - one in log viewer on the background and the other as JOptionPane message on foreground.

bkurotsu
Offline
Joined: 2004-12-13

Looks good, I will try to integrate it for you now.

bkurotsu
Offline
Joined: 2004-12-13

Never mind, it is rev 1547.

bkurotsu
Offline
Joined: 2004-12-13

One question - you are only catching the one type of exception. Perhaps it should be more like:

catch (FileNotFoundException) {}
catch (IOException) {}
catch (Exception) {}

etc? Something more robust...

ersh
Offline
Joined: 2006-10-18

There is the code a little below :

} catch (IOException ex) {
log.log(Level.SEVERE, "LogViewer report", ex);
} catch (SAXException ex) {
log.log(Level.SEVERE, "LogViewer report", ex);
}