New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WFCORE-2717] try-catch block doesn't work if catch block doesn't contains anything #2361
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank-you for this fix and the unit test!
@@ -57,6 +57,7 @@ static TryCatchFinallyControlFlow get(CommandContext ctx) { | |||
private int state; | |||
|
|||
TryCatchFinallyControlFlow(CommandContext ctx) { | |||
tryList = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could make tryList final and perhaps inline initialisation in declaration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doing so, any check for null in the class (I think there are some) should be removed.
@@ -97,6 +98,9 @@ boolean isInFinally() { | |||
} | |||
|
|||
void moveToCatch() throws CommandLineException { | |||
if(catchList == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could move the list initialisation in the switch (IN_TRY). Doing so, no need to check for null and you will only initialise the list when needed.
@@ -111,6 +115,9 @@ void moveToCatch() throws CommandLineException { | |||
} | |||
|
|||
void moveToFinally() throws CommandLineException { | |||
if(finallyList == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could move the list initialisation in the switch (IN_TRY and IN_CATCH). Doing so, no need to check for null and you will only initialise the list when needed.
Seems that you could get rid-off a call to break and merge the 2 cases.
Windows Build 3419 outcome was FAILURE using a merge of 323ab73 Failed tests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent! Thank-you.
JF
Windows Build 3420 outcome was FAILURE using a merge of 323ab73 Failed tests
|
retest this please! |
Core - Full Integration Build 4764 outcome was FAILURE using a merge of 323ab73 |
Full integration - Windows Build 3191 outcome was FAILURE using a merge of 323ab73 Failed tests
|
retest this please! |
Issue: https://issues.jboss.org/browse/WFCORE-2717