search

morfologik / morfologik-stemming

Tools for finite state automata construction and dictionary-based morphological dictionaries. Includes Polish stemming dictionary.
BSD 3-Clause "New" or "Revised" License
186 stars 44 forks source link

incorrect if logic in Speller.replaceRunOnWords()? #96

Closed danielnaber closed 6 years ago

danielnaber commented 6 years ago

This looks wrong, it uses getOutputConversionPairs() if it's empty, shouldn't it be the other way round?

https://github.com/morfologik/morfologik-stemming/blob/dc8dde7da215b51166931f3eb4d49a74397f8a01/morfologik-speller/src/main/java/morfologik/speller/Speller.java#L340-L345

I'm not sure if applyReplacements() makes sense at all. We've just checked that both parts are in the dictionary, should replacements still be applied? The if isn't tested, remove it and testRunonWords() still works.

Ping @milekpl, seems this was originally your code.

danielnaber commented 6 years ago

@jaumeortola Maybe you can comment on this?

jaumeortola commented 6 years ago

I think you are right. The 'if' can be removed and just do candidates.add( ). But I haven't done any test. Is this used anywhere? Does it cause any error?

danielnaber commented 6 years ago

I found this when trying to develop a fix for https://github.com/languagetool-org/languagetool/issues/731#issuecomment-328265948 and I just thought it's probably better to clean it up first.