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

Update & simplify goverter #17

Merged
merged 10 commits into from
Jan 11, 2024
Merged

Update & simplify goverter #17

merged 10 commits into from
Jan 11, 2024

Conversation

jmattheis
Copy link
Contributor

I've found your project while looking for projects using goverter. It's great
to see how it's used (:. I've upgraded it to the newest version and simplified
some stuff. See commit messages for explanations.

The last commit is a showcase how some custom code could be converted, I think
both solutions are valid, so I haven't changed this for every occurrence.

I've also created the feature request for regex goverter:map in
jmattheis/goverter#122 because this would simplify a
lot of goverter:map calls here.

Goverter can be execute with go generate ./db

Field settings may only be configured on struct or struct pointers.
See https://goverter.jmattheis.de/guide/configure-nested

With the newest goverter versios this is an error.
Goverter has features to prevent compile errors when generating the
converter into the same package. This commit basically follows the
example listed in the guide.

See https://goverter.jmattheis.de/guide/output-same-package
The output changed because the logic when goverter creates now methods
was changed. The implementation should still behave the same.
@sirgwain
Copy link
Owner

Wow! Thank you so much! I was looking for a code generator to copy my database domain objects into the game domain and goverter fit the bill perfectly! I noticed you had upgraded and incorporated a number of changes, but I hadn't gotten around to figuring out how to migrate yet. :)

Comment on lines +17 to +30
csFleet.MapObject = ExtendFleetMapObject((*source))
csFleet.FleetOrders = ExtendFleetFleetOrders((*source))
csFleet.PlanetNum = (*source).PlanetNum
csFleet.BaseName = (*source).BaseName
csFleet.Cargo = c.dbFleetToCsCargo((*source))
csFleet.Fuel = (*source).Fuel
csFleet.Age = (*source).Age
csFleet.Tokens = ShipTokensToGameShipTokens((*source).Tokens)
csFleet.Heading = ExtendFleetHeading((*source))
csFleet.WarpSpeed = (*source).WarpSpeed
csFleet.PreviousPosition = ExtendFleetPreviousPosition((*source))
csFleet.OrbitingPlanetNum = (*source).OrbitingPlanetNum
csFleet.Starbase = (*source).Starbase
csFleet.Spec = FleetSpecToGameFleetSpec((*source).Spec)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question on this generated code. Won't this create a copy of source for each variable assignment?

I think this pointer dereferencing makes a copy of the struct each time. I'm not sure if the go compiler optimizes that sort of thing away.

csFleet.PlanetNum = (*source).PlanetNum
csFleet.BaseName = (*source).BaseName
csFleet.Fuel = (*source).Fuel
csFleet.Age = (*source).Age

In the previous iteration, the pointer was only dereferenced once when calling

c.dbFleetToCsFleet((*source))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dereferencing does not copy source, dereferencing only gets the underlying value. The code above is actually what happens in the background when you access fields on a struct pointer in go. E.g.

type S struct{ Name string }

func main() {
	var s *S = nil
	_ = s.Name
	_ = *s
}

Both expressions s.Name and *s dereference s and they also both throw the same nil panic.

See https://stackoverflow.com/a/38443456/4244993

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, I didn’t realize that but it makes sense. Thanks!

@sirgwain sirgwain merged commit 880fa35 into sirgwain:develop Jan 11, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants