Github user osma commented on the pull request:
https://github.com/apache/jena/pull/101#issuecomment-158932591
Thanks @ajs6f - there's quite a lot of fixes in here!
In general, I feel positive about cleanups like this. And it's nice to have
a patch which actually *reduces* the amount of code quite significantly!
I'm a bit worried about the readability aspect here though. There are some
techniques in here that are unfamiliar to me - having learnt Java more than a
decade ago, around the time of JDK 1.4, and mostly avoiding Java in favor of
other programming languages ever since (though I have picked up some techniques
on the way at least up to Java 6). Using modern, clean and efficient techniques
is good, but how modern can you be and still have people understand the code?
For example, the way you replaced the TextHitConverter class with a
[forEach
expression](https://github.com/apache/jena/pull/101/files#diff-48122cd8c64b1e120b37581f50fc55ceR252)
- I didn't even know such a syntax existed! I do like getting rid of the
converter class, but is this a style we want to encourage? It looks a bit like
typical jQuery code to me, with all the (nested) callbacks...
What do others feel? @afs?
Also in terms of code review, it would be easier to take this one piece
(commit) at a time, with descriptive commit messages, instead of everything in
one lump.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
