rzo1 commented on code in PR #109:
URL: https://github.com/apache/opennlp-sandbox/pull/109#discussion_r1262906118
##########
opennlp-similarity/src/main/java/opennlp/tools/parse_thicket/apps/MinedSentenceProcessor.java:
##########
Review Comment:
Unused and therefore deleted?
##########
opennlp-dl/src/main/java/opennlp/tools/dl/NeuralDocCatTrainer.java:
##########
@@ -55,24 +55,24 @@ public class NeuralDocCatTrainer {
public static class Args {
@Option(name = "-batchSize", usage = "Number of examples in minibatch")
- int batchSize = 128;
+ final int batchSize = 128;
Review Comment:
Can `args4j` handle `final` values?
Judging from
https://github.com/kohsuke/args4j/blob/master/args4j/src/org/kohsuke/args4j/spi/Setters.java#L31
it might result in a (Runtime)exception?
##########
opennlp-similarity/src/main/java/opennlp/tools/similarity/apps/ContentGeneratorSupport.java:
##########
@@ -77,9 +77,12 @@ public static List<String>
buildSearchEngineQueryFromSentence(String sentence) {
boolean bAccept = true;
for (String w : qs) {
if (w.toLowerCase().equals(w)) // idf
only two words then
- // has to be person name,
- // title or geolocation
+ // has to be person name,
+ // title or geolocation
+ {
bAccept = false;
+ break;
Review Comment:
If we break early, we can remove `bAccept` here? It is only used for a
`continue` later, which is obsolete if we break early.
##########
opennlp-similarity/src/main/java/opennlp/tools/similarity/apps/RelatedSentenceFinder.java:
##########
@@ -217,9 +217,12 @@ public static List<String>
buildSearchEngineQueryFromSentence(String sentence) {
boolean bAccept = true;
for (String w : qs) {
if (w.toLowerCase().equals(w)) // idf
only two words then
- // has to be person name,
- // title or geolocation
+ // has to be person name,
+ // title or geolocation
+ {
bAccept = false;
Review Comment:
same as above
##########
opennlp-similarity/src/main/java/opennlp/tools/similarity/apps/ContentGenerator.java:
##########
@@ -146,9 +146,12 @@ public static List<String>
buildSearchEngineQueryFromSentence(String sentence) {
boolean bAccept = true;
for (String w : qs) {
if (w.toLowerCase().equals(w)) // idf
only two words then
- // has to be person name,
- // title or geolocation
+ // has to be person name,
+ // title or geolocation
+ {
bAccept = false;
+ break;
Review Comment:
If we break early, we can remove `bAccept` here? It is only used for a
`continue` later, which is obsolete if we break early.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
