use java 7 features #819
use java 7 features #819
Conversation
Codecov Report
@@ Coverage Diff @@
## master #819 +/- ##
===========================================
+ Coverage 74.54% 74.65% +0.1%
+ Complexity 3334 3323 -11
===========================================
Files 364 364
Lines 10354 10316 -38
Branches 1298 1294 -4
===========================================
- Hits 7718 7701 -17
+ Misses 2171 2150 -21
Partials 465 465
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
In summary this looks quite good to me!
The code deleted in the tests corresponds to the deleted methods in the utility classes.
One minor thing might be the exception handling replaced by ReflectiveOperationException, but I think this is more a matter of taste.
| @@ -51,7 +51,7 @@ public Object intercept(Object target, Method method, Object[] arguments, IRespo | |||
|
|
|||
| try { | |||
| return method.invoke(delegate, arguments); | |||
| } catch (IllegalAccessException | InvocationTargetException e) { | |||
| } catch (ReflectiveOperationException e) { | |||
There was a problem hiding this comment.
I would favour the previous version of this, because using ReflectiveOperationException looks to me like Pokemon exception handling. Instead of catching only the specific exceptions that can be thrown, now all exceptions (even unexpected ones / error condition) are caught.
| @@ -17,47 +17,27 @@ package org.spockframework.util | |||
| import spock.lang.Specification | |||
|
|
|||
| class ObjectUtilSpec extends Specification { | |||
| def "null aware equals"() { | |||
There was a problem hiding this comment.
- Was also deleted from
ObjectUtil
|
|
||
| package org.spockframework.util | ||
|
|
||
| import spock.lang.* | ||
|
|
||
| class CollectionUtilSpec extends Specification { | ||
| def "copy an array"() { |
There was a problem hiding this comment.
- Was also deleted from
CollectionUtil
| @@ -134,7 +135,7 @@ class HtmlReportGenerator { | |||
| def target = new File(outputDirectory, resourcePath) | |||
| if (!target.exists()) { | |||
| target.parentFile.mkdirs() | |||
| IoUtil.copyStream(source, new FileOutputStream(target)) | |||
There was a problem hiding this comment.
- Was also deleted from
IoUtil
|
Thanks @AlexElin, FWIW the PR came at an time where Spock was majorly refactored and we didn't accept merge requests. The changes proposed here have also been made in other PRs so I'm closing it after 4 years 😅. |
Migrate code to using features of Java 7 like try-with-resources, Objects class and etc
This change is