Remove use of System.exit() calls from codebase

allenai / openie-standalone

Quality information extraction at web scale. Edit

Other

327 stars 52 forks source link

Remove use of System.exit() calls from codebase #20

Closed lewismc closed 5 years ago

lewismc commented 7 years ago

I'm currently working on building openie into Apache Any23 via https://github.com/apache/any23/pull/33, a major issue I am having however is with Unit testing the code I'm attempting to implement. The is due to the Maven surefire plugin failing whenever it encounters System.exit(). I've scanned the openie codebase and there appears to be a few places where this is the case. I am not 100% sure that this stems from openie, however I will begin by submitting the PR here to narrow the issue down.

lewismc commented 7 years ago

@Yongyao CC for context

jkinkead commented 7 years ago

The Relnoun case is definitely an error; that should be throwing an exception or similar.

The Nesty case is in a main method, so it seems appropriate.

lewismc commented 7 years ago

OK thanks @jkinkead I'm working on a PR and testing locally to see if this resolves my issue.

The Nesty case is in a main method, so it seems appropriate.

Two things spring to mind

is there any reason for returning the non-zero code?
Seeing as we are within main this would be all the more reason to not use System.exit(). Main will terminate after the exception message has been thrown.

aimichal commented 5 years ago

We're not actively working on making changes to this codebase at the moment. If you have a suggestion for what to change, a PR would be welcome. (I agree that deeply nested library code shouldn't make System.exit() calls.)

Closing this issue now as part of cleanup, but please re-open if there's more to discuss.