Is there any particular reason for flipping the imports to wildcards? We
tend to import the individual classes elsewhere in the codebase. My
personal preference would be to leave those as individual imports. It
doesn't make the import list that much longer, and I find its clearer what
is being imported and used. IDEs tend to fold those away unless you're
specifically looking at them too.
I think like the other changes, and do think it looks neater and easier to
read.
I personally prefer
map = new HashMap<>();
over
map = new HashMap<Object, Object>();
but I don't think that's a big deal.
I also note this change:
- moduleLocations.removeIf(file -> !match((String) modules,
file));
-
+ for (final Iterator<File> i = moduleLocations.iterator();
i.hasNext(); ) {
+ final File file = i.next();
+ if (!match((String) modules, file)) {
+ i.remove();
+ }
+ }
Which is changing from a lambda to a loop. I'd probably have done it like
this, but the effect is the same:
final Iterator<File> iterator = moduleLocations.iterator();
while (iterator.hasNext()) {
final File file = iterator.next();
if (!match((String) modules, file)) {
iterator.remove();
}
}
IntelliJ is definitely preferring the moduleLocations.removeIf, and will
refactor both of those loops to it with one keystroke. I have the what is
probably an unpopular opinion which is that I actually prefer the loop over
the lambda. The reasons are:
1. Backporting changes is much harder when you have to work around language
changes like this
2. I personally find lots of things crammed into one line harder to read,
and in this particular case, there's a cast and a negation (so a double
negative) in the removeIf.
Jon
On Mon, Dec 31, 2018 at 10:21 AM Jonathan Gallimore <
[email protected]> wrote:
> Yep, I'll take a look. Thanks for the PR.
>
> Jon
>
> On Mon, Dec 31, 2018 at 9:10 AM Gurkan Erdogdu <[email protected]>
> wrote:
>
>> Hi
>> I have created a PR (https://github.com/apache/tomee/pull/332) for
>> (TOMEE-2435)
>> Simplify the Code in org.apache.openejb.OpenEjbContainer$Provider
>> <https://issues.apache.org/jira/browse/TOMEE-2435>
>>
>> If anybody can check and merge, it is appreciated :)
>> Regards.
>> Gurkan
>>
>
