SUNET icon indicating copy to clipboard operation
SUNET copied to clipboard

consider dependency injection

Open paulpach opened this issue 7 years ago • 0 comments

Consider using dependency injection. You have code like this:

int TextStrategy::_init() {
    wordseg = WordSeg::_get_instance();
    if (NULL == wordseg) {
        FATAL_LOG("Init Wordseg Error!");
        return SUB_FAIL;
    }
    std::string crf_model_path = "";
    SubConfig::_get_instance()->_get_conf_val("crf_model", crf_model_path);
    if (! wordseg->_init_tagger(crf_model_path.c_str())) {
        FATAL_LOG("Init Tagger Error!");
        return SUB_FAIL;
    }
    DEBUG_LOG("Init TextStrategy Okay!");
    return SUB_OK;
}

The issue here is you are hard coding your dependencies, TextStrategy will always use that one implementation of WordSeg and SubConfig.

If you use dependency injection, the dependencies would be passed to TextStrategy typically in the constructor, and you would receive an abstract class. That way someone using TextStrategy could pass any implementation they want, they just have to implement the abstract class.

I don't like my classes calling something to get their configuration values. It makes it difficult if you want to pass your configuration from other sources such as environment variables or command line arguments. It makes it hard if you want multiple instances of your class using different configuration. Instead the configuration values should be passed to your class either in the constructor or in a setter method. Let whomever uses your class figure out how to get the configuration for your class.

paulpach avatar Jun 15 '17 12:06 paulpach