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

Improving string2code conversion #65

Closed
lidel opened this issue Mar 24, 2022 · 2 comments
Closed

Improving string2code conversion #65

lidel opened this issue Mar 24, 2022 · 2 comments

Comments

@lidel
Copy link
Member

lidel commented Mar 24, 2022

https://github.com/ipfs/go-cid/pull/137/files is removing hardcoded mapping tables from go-cid,
and we need a replacement for people who will be migrating to go-multicodec

I want to document the best way way of doing string → code and code → string

My understanding is that:

Old way (go-cid)

cid "github.com/ipfs/go-cid"

// string → code
code := cid.Codecs["libp2p-key"]

// code → string
string := cid.CodecToStr[code]

New way (go-multicodec)

mc "github.com/multiformats/go-multicodec"

// string → code
var mc.Code code 
code.Set("libp2p-key")

// code → string
code.String()

@mvdan any thoughts if this is good enough?
Should we add StrToCodec and CodecToStr maps to go-multicodec, or is it not worth it?

@mvdan
Copy link
Contributor

mvdan commented Mar 25, 2022

I'd prefer to not expose two ways of doing the same thing in the API. Is there a particular reason why you want the maps?

One big advantage with the method we chose is that it uses widespread interfaces (https://pkg.go.dev/fmt#Stringer and https://pkg.go.dev/flag#Value), so a multicodec.Code will work out of the box with fmt.Println and flag.Var. Another advantage is that methods are tied to the type, whereas global maps aren't - and they could be modified as well, since Go doesn't have read-only maps.

So, overall, I'd ask if there's a good reason that what we have right now isn't enough :) It's true that "string to code" is one more line now, but you could also draw lines in the opposite direction, such as fmt.Println(cid.CodecToStr[code]) now being just fmt.Println(code) thanks to the String method.

@lidel
Copy link
Member Author

lidel commented Mar 25, 2022

Thank you @mvdan!

In this context keeping it as-is makes sense, I will document it, no need to add anything new.

@lidel lidel closed this as completed Mar 25, 2022
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

2 participants