drgomesp / cargo

:package: A Go dependency injection container that promotes low coupling and inversion of control
MIT License
34 stars 2 forks source link

added Container.MustGet() function #24

Closed vgarvardt closed 8 years ago

drgomesp commented 8 years ago

Thanks for the pull request, @vgarvardt.

Although I appreciate your contribution, I'm having trouble finding a reason for a MustGet method.

We can achieve the same results with

service, err := container.Get("foo)

if err !=nil {
    panic(err)
}

I don't think the container should panic if a service is not found.

vgarvardt commented 8 years ago

It's just wrap for the code above to avoid code duplications - e.g. in my case missed id in container alway must panic, so MustGet saves 4 lines of code for each usage. Same as html/template.Must() and many other examples.

drgomesp commented 8 years ago

I understand your point, but my question (which is more of a rhetoric question) is: does the container have the responsibility of panicking if a service is not found?

For example, when you hit a web page the doesn't exist, does the response comes as 500 or 404?

tomsquest commented 8 years ago

@drgomesp I do think the container must panic when an asked dependency is not found, or better the asker can choose, via Get and MustGet.

vgarvardt commented 8 years ago

For example, when you hit a web page the doesn't exist, does the response comes as 500 or 404?

For example when you hit a config loader or any other important resource that does not exist.

vgarvardt commented 8 years ago

Added couple tests.

drgomesp commented 8 years ago

Thank you, @vgarvardt!