0

New Gopher here, coming from Java land.

Let's say I have a some generic storage interface:

package repositories

type Repository interface {
  Get(key string) string
  Save(key string) string
}

I support multiple different backends (Redis, Boltdb, etc) by implementing this interface in separate packages. However, each implementation has unique configuration values that need to be passed in. So I define a constructor in each package, something like:

package redis 

type Config struct {
 ...
}

func New(config *Config) *RedisRepository {
  ...
}

and

package bolt

type Config struct {
  ...
}

func New(config *Config) *BoltRepository {
  ...
}

main.go reads a json configuration file that looks something like:

type AppConfig struct {
  DatabaseName string,
  BoltConfig *bolt.Config,
  RedisConfig *redis.Config,
}

Based on the value of DatabaseName, the app will instantiate the desired repository. What is the best way to do this? Where do I do it? Right now I'm doing some kind of horrible factoryfactory method which seems very much like a Go anti-pattern.

in my main.go, I have a function that reads the above reflected configuration values, selecting the proper configuration (either BoltConfig or RedisConfig) based on the value of DatabaseName:

func newRepo(c reflect.Value, repoName string) (repositories.Repository, error) {
    t := strings.Title(repoName)

    repoConfig := c.FieldByName(t).Interface()

    repoFactory, err := repositories.RepoFactory(t)
    if err != nil {
        return nil, err
    }

    return repoFactory(repoConfig)
}

and in my repositories package, I have a factory that looks for the repository type and returns a factory function that produces an instantiated repository:

func RepoFactory(provider string) (RepoProviderFunc, error) {
    r, ok := repositoryProviders[provider]
    if !ok {
        return nil, fmt.Errorf("repository does not exist for provider: %s", r)
    }
    return r, nil
}

type RepoProviderFunc func(config interface{}) (Repository, error)

var ErrBadConfigType = errors.New("wrong configuration type")

var repositoryProviders = map[string]RepoProviderFunc{
    redis: func(config interface{}) (Repository, error) {
        c, ok := config.(*redis.Config)
        if !ok {
            return nil, ErrBadConfigType
        }
        return redis.New(c)
    },
    bolt: func(config interface{}) (Repository, error) {
        c, ok := config.(*bolt.Config)
        if !ok {
            return nil, ErrBadConfigType
        }
        return bolt.New(c)
    },
}

bringing it all together, my main.go looks like:

cfg := &AppConfig{}
err = json.Unmarshal(data, cfg)
if err != nil {
    log.Fatalln(err)
}

c := reflect.ValueOf(*cfg)

repo, err := newRepo(c, cfg.DatabaseName)
if err != nil {
    log.Fatalln(err)
}

And yes, the second I was done typing this code I recoiled at the horror I had brought into this world. Can someone please help me escape this factory hell? What's a better way to do this type of thing -i.e selecting an interface implementation at runtime.

user7467314
  • 725
  • 1
  • 6
  • 10
  • 2
    In broad strokes, the approach in the question is similar to how the database/sql and image packages handle this issue. See the implementations of [Open](https://godoc.org/database/sql#Open) / [Register](https://godoc.org/database/sql#Register) in database/sql. – Charlie Tumahai Mar 29 '17 at 02:29
  • 2
    Related / similar question: [Registering packages in Go without cyclic dependency](http://stackoverflow.com/questions/29271440/registering-packages-in-go-without-cyclic-dependency). – icza Mar 29 '17 at 03:21
  • The problem originally arose because I had a cyclical dependency -very helpful links guys, thanks. – user7467314 Mar 29 '17 at 18:26

1 Answers1

2

Do you need dynamic registration? It seems like the list of backends is already baked into your server because of the AppConfig type, so you may be better just writing the simplest possible factory code:

func getRepo(cfg *AppConfig) (Repository, error) {
    switch cfg.DatabaseName {
    case "bolt":
        return bolt.New(cfg.BoltConfig), nil
    case "redis":
        return redis.New(cfg.RedisConfig), nil
    }
    return nil, fmt.Errorf("unknown database: %q", cfg.DatabaseName)
}

func main() {
    ...
    var cfg AppConfig
    if err := json.Unmarshal(data, &cfg); err != nil {
        log.Fatalf("failed to parse config: %s", err)
    }
    repo, err := getRepo(&cfg)
    if err != nil {
        log.Fatalln("repo construction failed: %s", err)
    }
   ...

}

Sure, you can replace this with generic reflection-based code. But while that saves a few lines of duplicated code and removes the need to update getRepo if you add a new backend, it introduces a whole mess of confusing abstraction, and you're going to have to edit code anyway if you introduce a new backend (for example, extending your AppConfig type), so saving a couple of lines in getRepo is hardly a saving.

It might make sense to move getRepo and AppConfig into a repos package if this code is used by more than one program.

Paul Hankin
  • 54,811
  • 11
  • 92
  • 118
  • I think you're right, i'm just overcomplicating things. It would make sense to have a registry/factory and dynamically configuration repos if this were a library, but since all supported backends are already 'baked in' to the server as you say, there really is no point. – user7467314 Mar 29 '17 at 18:25