search

EdwardTang / clearnlp

Automatically exported from code.google.com/p/clearnlp
Other
0 stars 1 forks source link

EngineGetters should throw exception instead of returning null #1

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
I was looking at the static factory methods in EngineGetter, and I noticed 
several methods like this:

    static public AbstractSegmenter getSegmenter(String language, AbstractTokenizer tokenizer)
    {
        if (language.equals(AbstractReader.LANG_EN))
            return new EnglishSegmenter(tokenizer);

        return null;
    }

It seems that instead of returning null, these methods should return an 
IllegalArgumentException that says "the requested language is not currently 
supported".

Original issue reported on code.google.com by lee.becker on 27 Oct 2012 at 5:33

GoogleCodeExporter commented 9 years ago 
Lee,

Good point; the exceptions are added for the getC2DConverter, getTokenizer, and 
getSegmenter methods under EnginGetter.  Thanks!

Jinho

Original comment by jdcho...@gmail.com on 29 Oct 2012 at 11:11

GoogleCodeExporter commented 9 years ago 
Similar problems exist for the POS tagger and Dep Parser factory methods. The 
patch fixes them I think

(the patch is just taken from the 'fix-deppred-npe' branch of my clone: 
https://code.google.com/r/admackin-clearnlp-spans/source/list?name=fix-deppred-n
pe . I wish this was github/bitbucket so I could just issue a pull request)

Original comment by admac...@gmail.com on 23 Jan 2013 at 7:14

Attachments: