Skip to content

Unclear Get(...) API and documentation #26

@nielskrijger

Description

@nielskrijger

Hi there! I tried using the library but got a little confused.

Particularly the current documentation and API of .Get(...) was a matter of trial & error.

// Get returns the tag associated with the given key. If the key is present
// in the tag the value (which may be empty) is returned. Otherwise, the
// returned value will be the empty string. The ok return value reports whether
// the tag exists or not (which the return value is nil).
func (t *Tags) Get(key string) (*Tag, error) {
	for _, tag := range t.tags {
		if tag.Key == key {
			return tag, nil
		}
	}

	return nil, errTagNotExist
}

The comment suggests a pattern like:

docTag, ok := tags.Get("documentation)
if ok {
 ...
}

In reality, ok is an error, not a boolean, so it wouldn't work.

The main README adds to the confusion:

// ... and start using structtag by parsing the tag
jsonTag, err := tags.Get("json")
if err != nil {
    panic(err)
}

Based on that panic I wouldn't expect the only error returned by Get is errTagNotExist (which perfectly OK in my scenario).

The panic example in the docs signals something more severe is going on. Moreover, because errTagNotExist is not exported, callers can’t do something like

if errors.Is(err, structtag.ErrTagNotExist) { … }

A more sensible signature would be:

docTag, ok := tags.Get("documentation")
if ok {
    // …
}

so that the caller can check existence with a boolean rather than treating “not found” as an error. Similar to Go's type checking. Unfortunately, changing Get from (*Tag, error) to (*Tag, bool) would break backwards compatibility.

As an alternative, the library could introduce something like:

func (t *Tags) Find(key string) (*Tag, bool)

that returns a boolean found flag instead of an error. This would leave Get unchanged (preserving compatibility) while providing a clearer, more “Go-idiomatic” lookup method.

There's various possibilities here, not sure which is the best one. I figured I'd just share my first experience with this lib :-)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions