do icon indicating copy to clipboard operation
do copied to clipboard

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

Open startan opened this issue 2 years ago • 6 comments

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))
}

startan avatar Feb 13 '23 14:02 startan

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.

kentquirk avatar Feb 13 '23 17:02 kentquirk

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

flymedllva avatar Feb 14 '23 22:02 flymedllva

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

jeeo avatar Feb 17 '23 13:02 jeeo

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.

btvoidx avatar Jun 17 '23 16:06 btvoidx

@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
}

wregen avatar Sep 04 '23 18:09 wregen

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.

jProgr avatar Nov 23 '23 23:11 jProgr