juji-io / symspell-clj

SymSpell spell checker in Clojure
MIT License
22 stars 1 forks source link

get-suggestion breaks when lookup returns empty vector #2

Closed den1k closed 1 year ago

den1k commented 1 year ago
(def checker (sp/new-spellchecker))

(sp/get-suggestion checker "nonsense")
; => "nonsense"
(sp/get-suggestion checker "nonsenseXYZ")
; => 
; Execution error (NullPointerException) at symspell-clj.core/normalize-suggestion (core.clj:110).
; null

the issue is that the code tries to call .getSuggestion on nil

https://github.com/juji-io/symspell-clj/blob/main/src/symspell_clj/core.clj#L126-L129

https://github.com/juji-io/symspell-clj/blob/main/src/symspell_clj/core.clj#L108-L110

I was working on a fix but then realized I'm not sure what the API should be. When no suggestion could be found, should get-suggestion return the original input or nil or ""? If it should return nil, then an "" should probably as well. Currently: (sp/get-suggestion checker "") => ""

Here's a tweak of spell-check that returns the original input:

(defn- spell-check
  [spell-checker input]
  (cond
    (= " " input) input
    (non-word? input) input
    :else (let [capitalized? (captialized-input? input)
                all-cap?     (all-cap-input? input)
                input'       (s/lower-case input)
                suggestion   (first (if (re-find #" " input')
                                      (lookup-compound spell-checker input')
                                      (lookup spell-checker input')))]
            (if suggestion
              (normalize-suggestion suggestion all-cap? capitalized?)
              input))))
huahaiy commented 1 year ago

I would think it make sense to return nil, since Clojure people are used to do nil-punning, so get-x return nil when x doesn't exist. Return something else would be surprising?

den1k commented 1 year ago

Okay, should and empty string "" then also return nil?

huahaiy commented 1 year ago

I think so?