Hi Jon I will update my patch to update wildcard import, loop and diamond <> operator. Regards Gurkan
On Mon, Dec 31, 2018 at 2:38 PM Jonathan Gallimore < [email protected]> wrote: > 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 > >> > > >
