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

modelgen: Generate getter/setter functions for structs #211

Open
dave-tucker opened this issue Aug 6, 2021 · 0 comments
Open

modelgen: Generate getter/setter functions for structs #211

dave-tucker opened this issue Aug 6, 2021 · 0 comments

Comments

@dave-tucker
Copy link
Collaborator

dave-tucker commented Aug 6, 2021

In working on #191 it became clear that optional fields (i.e those that are pointers) are annoying to work with.
OVSDB Schema's also declare a number of fields immutable and as such it would be best to prevent users from being able to mutate them and expect a result.

Both of these problems could be fixed by using getter/setters. The pattern (which is still idiomaitc Go, even though it feels very object oriented) for this would be as follows:

type Foo struct {
    uuid        string
    optional *string
}

func (f *Foo) UUID() string {
    return f.uuid
}

func (f *Foo) Optional() *string {
    return f.optional
}

func (f *Foo) SetOptional(s string) {
    f.optional = s
}

There are a few issues with this. Firstly, we can't marshal these structs to JSON, as it only supports exported fields.
Secondly, we can only set exported fields using reflection, which is used in the mapper layer 🤔

To combat the first issue, I propose that we follow the pattern that encoding/json uses and effectively consider our ovsdb package to be encoding/ovsdb. We would offer two functions, ovsdb.Marshal and ovsdb.Unmarshal. These operate using ovsdb: struct tags to populate a struct from [[bytes or to serialize a struct in to []bytes.

Alternatively, structs passed in to these functions must honour the interface:

type Marshaler interface {
	MarshalOVSDB() ([]byte, error)
}

type Unmarshaler interface {
        UnmarshalOVSDB([]byte) error
}

I'm hopeful perhaps that some of what mapper does could make it's way in to ovsdb package.

As for the second issue, we'd need to search for the setter function of a given field and invoke it with an argument, which certainly isn't a straightforward swap.

These would be massive changes to the architecture of libovsdb and would require a lot of discussion....

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

1 participant