Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it true that this library doesn't use the reflection package? You lied. #30

Open
startan opened this issue Feb 13, 2023 · 6 comments

Comments

@startan
Copy link

startan commented Feb 13, 2023

I thought there was some new hack to implement dependency injection without reflection and only through generics. A quick look at the code revealed that this was a lie.

// service.go:27
func generateServiceName[T any]() string {
	var t T

	// struct
	name := fmt.Sprintf("%T", t)   // <-- Here you use reflection !!!
	if name != "<nil>" {
		return name
	}

	// interface
	return fmt.Sprintf("%T", new(T))
}
@kentquirk
Copy link

It's this kind of PR (title, content) that drives people away from open source contributions.

Please consider the possibility that the author didn't deliberately lie. It would have been approximately 1000x friendlier and more useful to have said "hey, %T actually does reflection under the hood" and, ideally, offer a PR to change either the docs or the implementation.

@flymedllva
Copy link

Is there no way to use the full path to the package from go.mod in the service name instead of just the package in which it is located?
If you try to make Provide services with the same name and package in different subpackages it won't work
p1/p.Service
p2/p.Service

@jeeo
Copy link

jeeo commented Feb 17, 2023

I totally agree with @kentquirk.

If someone else gets curious about the details of how it's using reflection (like I was), there's the piece of code (from fmt package)
https://cs.opensource.google/go/go/+/refs/tags/go1.20.1:src/fmt/print.go;drc=1a9893a969e0a73dc4f1e48ed40ccaf29ec238a6;l=698

@btvoidx
Copy link

btvoidx commented Jun 17, 2023

There actually is a trick you can use to avoid reflection on map/context keys and it's called 0-width generic structs.

type key[T any] struct{}

Instances of such structs compare unequal if their T differs, such that key[string]{} != key[int]{} and, as far as I know, they still take up 0 bytes. These are perfectly fine to use as a context or a map key, provided map has interface key type. I use this trick in my event emitter package: https://github.com/btvoidx/mint.

I would like to implement it and open a PR, but these reflected names are used as part of package public api (see (*Injector).ListProvidedServices and related), so getting rid of them as map keys would not get rid of reflection, as type names are needed anyways, so this issue would still stand.

@wregen
Copy link

wregen commented Sep 4, 2023

@btvoidx: Please implement Injector with type key[T any] struct{}. I'm sure many people would be interested. Regarding changing the API: there is always the option to create github.com/samber/do@v2.

Minimal/global/simplified version could look like this:

package do

import (
	"errors"
)

type key[T any] struct{ name string }

type provider[T any] func() (T, error)

var services = make(map[any]any)

func ProvideNamed[T any](name string, p provider[T]) {
	services[key[T]{name}] = p
}

func ProvideNamedValue[T any](name string, v T) {
	services[key[T]{name}] = v
}

func InvokeNamed[T any](name string) (T, error) {
	provider, ok := services[key[T]{name}].(provider[T])
	if ok {
		return provider()
	}

	value, ok := services[key[T]{name}].(T)
	if ok {
		return value, nil
	}

	return empty[T](), errors.New("not found")
}

func Provide[T any](p provider[T]) {
	ProvideNamed[T]("", p)
}

func ProvideValue[T any](v T) {
	ProvideNamedValue[T]("", v)
}

func Invoke[T any]() (T, error) {
	return InvokeNamed[T]("")
}

func empty[T any]() (t T) {
	return
}

@jProgr
Copy link

jProgr commented Nov 23, 2023

I think it is rather dishonest to still claim in the README to not use reflection when it has been pointed out that it does in fact use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants