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

Generate functions directly without Converter interface #77

Open
jmattheis opened this issue Jun 3, 2023 · 9 comments
Open

Generate functions directly without Converter interface #77

jmattheis opened this issue Jun 3, 2023 · 9 comments
Labels
feature New feature or request

Comments

@jmattheis
Copy link
Owner

jmattheis commented Jun 3, 2023

Goverter generates an implementation for a user defined interface with conversion methods. The usability of that could be better, because now the user has to initialize the Converter struct and call methods from there. Goverter could generate functions directly. If this ticket will be included into Goverter, then the old interface approach will most likely be removed.

Current example (click me)

// goverter:converter
// goverter:extend ConvertAnimals
type Converter interface {
    Convert(source Input) Output

    // used only in extend method
    ConvertDogs([]Dog) []Animal
    ConvertCats([]Cat) []Animal
}

type Input struct {
    Animals InputAnimals
}
type InputAnimals struct {
    Cats []Cat
    Dogs []Dog
}
type Output struct {
    Animals []Animal
}

type Cat struct { Name string }
type Dog struct { Name string }

type Animal struct { Name string }

func ConvertAnimals(c Converter, input InputAnimals) []Animal {
    dogs := c.ConvertDogs(input.Dogs)
    cats := c.ConvertCats(input.Cats)
    return append(dogs, cats...)
}

Proposed approach

// goverter:func
// goverter:map Animals | CombineAnimals
var ConvertInput func (source *Input) *Output

// goverter:func
var ConvertDogs func([]Dog) []Animal

// goverter:func
var ConvertCats func([]Cat) []Animal

func CombineAnimals(input Input) []Animal {
	dogs := ConvertDogs(input.Dogs)
	cats := ConvertCats(input.Cats)
	return append(dogs, cats...)
}

type Input struct {
	Cats []Cat
	Dogs []Dog
}
type Output struct { Animals []Animal }
type Cat struct{ Name string }
type Dog struct{ Name string }
type Animal struct{ Name string }

Variables marked with goverter:func would allow all configs that can be added to the conversion methods on interfaces. Global goverter flags like goverter:extend could be configured on the package declaration.

When executing goverter on this input file, it could generate an init() method and set the generated implementation. The generated code for the example above could look like this:

Generated Code Example (click me)

func init() {
	ConvertInput = func(source *Input) *Output {
		var pStructsOutput *Output
		if source != nil {
			var structsOutput Output
			structsOutput.Animals = CombineAnimals((*source))
			pStructsOutput = &structsOutput
		}
		return pStructsOutput
	}

	ConvertCats = func(source []Cat) []Animal {
		var structsAnimalList []Animal
		if source != nil {
			structsAnimalList = make([]Animal, len(source))
			for i := 0; i < len(source); i++ {
				structsAnimalList[i] = structsCatToStructsAnimal(source[i])
			}
		}
		return structsAnimalList
	}

	ConvertDogs = func(source []Dog) []Animal {
		var structsAnimalList []Animal
		if source != nil {
			structsAnimalList = make([]Animal, len(source))
			for i := 0; i < len(source); i++ {
				structsAnimalList[i] = structsDogToStructsAnimal(source[i])
			}
		}
		return structsAnimalList
	}
}

func structsCatToStructsAnimal(source Cat) Animal {
	var structsAnimal Animal
	structsAnimal.Name = source.Name
	return structsAnimal
}
func structsDogToStructsAnimal(source Dog) Animal {
	var structsAnimal Animal
	structsAnimal.Name = source.Name
	return structsAnimal
}

Testable version available: #77 (comment)

Please 👍 this issue if you want this functionality. If you have a specific use-case in mind, feel free to comment it.

@jmattheis jmattheis added the feature New feature or request label Jun 3, 2023
@zhenzou
Copy link

zhenzou commented Nov 8, 2023

Hi jmattheis, thanks for your great project.
And I like your new proposed approach very much, would you like to share the progress on this feature?

@jmattheis
Copy link
Owner Author

Hey @zhenzou, there is no progress on this feature.

@KScaesar
Copy link

KScaesar commented Nov 9, 2023

I think we can keep the original Converter interface.
We just need to change the generated code implementation from a struct to a function.
The function name can be created by combining interface name + method name.

The downside is that the interface won't have an implementation, which might seem a bit odd.

@jmattheis
Copy link
Owner Author

@KScaesar Only doing this would prevent the feature of using generated methods in custom methods. E.g. https://goverter.jmattheis.de/#/config/extend?id=method-with-converter

// goverter:converter
// goverter:extend ConvertAnimals
type Converter interface {
    Convert(source Input) Output

    // used only in extend method
    ConvertDogs([]Dog) []Animal
    ConvertCats([]Cat) []Animal
}

func ConvertAnimals(c Converter, input InputAnimals) []Animal {
    dogs := c.ConvertDogs(input.Dogs)
    cats := c.ConvertCats(input.Cats)
    return append(dogs, cats...)
}

Both the function and interface definition should support the same featureset.

@peanutgyz
Copy link

peanutgyz commented Nov 16, 2023

I think we can keep the original Converter interface.

// goverter:converter
// goverter:extend ConvertAnimals
type Converter interface {
    Convert(source Input) Output

    // used only in extend method
    ConvertDogs([]Dog) []Animal
    ConvertCats([]Cat) []Animal
}

func ConvertAnimals(input InputAnimals) []Animal {
    dogs := ConvertDogs(input.Dogs)
    cats := ConvertCats(input.Cats)
    return append(dogs, cats...)
}```

@jmattheis
Copy link
Owner Author

At the time of generation ConvertDogs and ConvertCats doesn't exist. So the code won't successfully compile. Goverter uses type information for generation and therefore requires that the package with the converter interface compiles.

@jmattheis
Copy link
Owner Author

I'm looking for testers of the new style for generation. The version should work well, but they likely will some changes before this will be released.

Install

  • go install github.com/jmattheis/goverter/cmd/goverter@variables-v3
  • or go run github.com/jmattheis/goverter/cmd/goverter@variables-v3 [pattern]
  • or go get github.com/jmattheis/goverter@variables-v3

Usage

When using the version above, you can declare goverter:variables on a var statement containing multiple variables of type func (Source) Target. When running goverter, it will create a new file in the same package with an func init initializing the defined variables.

You can define Converter settings directly on the outer var statement. Methods settings can be defined on the separate variables.

Here an complex example in both styles.

Interface/struct style

// goverter:converter
// goverter:extend SQLStringToPString
type Converter interface {
	ConvertHouse(source DBHouse) APIHouse
	// goverter:map Name FirstName
	// goverter:ignore Age
	ConvertPerson(source DBPerson) APIPerson
	// goverter:map Owner.Name OwnerName
	ConvertApartment(source DBApartment) APIApartment
}

Variables style

// goverter:variables
// goverter:extend SQLStringToPString
var (
	ConvertHouse func(source DBHouse) APIHouse
	// goverter:map Name FirstName
	// goverter:ignore Age
	ConvertPerson func(source DBPerson) APIPerson
	// goverter:map Owner.Name OwnerName
	ConvertApartment func(source DBApartment) APIApartment
)

If you've tried out the new style, it would be great if you could answer the following questions:

  • How do you like the new style?
  • Have you found any bugs when using the new style?
  • Is there a feature missing in the new style to make it usable to you?
  • Do you have an idea how both styles could be integrated into the documentation?

@ryan961
Copy link

ryan961 commented Apr 10, 2024

Hi, thanks for this amazing tool. I'm currently trying out the new variables approach for generating converters, and it's a significant improvement compared to the original converter.

I've been using goverter for a while now (#100), and the original converter was indeed very useful. However, there was a minor issue where once I deleted converter_gen.go and then ran goverter gen .. again, it would fail. (I believe there was an issue in older versions where changing the field types would prevent regeneration without deleting converter_gen.go. Apologies for not investigating and tracking the issue further.)

To work around this, I defined the following function:

func newConverter() converter {
	return &converterImpl{}
}

When generating:

func newConverter() converter {
	return nil
}

This function helped minimize changes when deleting and regenerating.

The new variables approach completely eliminates the hassle of regenerating after deleting converter_gen.go. I hope it can be merged into the main branch soon.

@grachevko
Copy link

Looks great!
In my case no bugs found.

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

No branches or pull requests

6 participants