search

geographiclib / geographiclib

Main repository for GeographicLib
MIT License
289 stars 70 forks source link

Added move constructor/assignment support for MagneticModel/GravityModel #28

Closed ashbob999 closed 3 months ago

ashbob999 commented 3 months ago

Since the move constructor/assignment functions cannot be implicitly defined, I have added them as default, so that the MagneticModel class can be moved.

cffk commented 3 months ago

I don't remember why I deleted the copy and assignment constructors; and, now, I don't see why they should be deleted. So perhaps the simpler solution for you would be to allow the defaults for these constructors? Would that work? (Also whatever is done to MagneticModel should be done also to GravityModel.)

ashbob999 commented 3 months ago

I agree, it should work because the deleted functions are disabling the trivial ones. Do removing then all would be fine. I'll modify and test tomorrow.

ashbob999 commented 3 months ago

On further inspection it looks like they need to stay deleted. As the MagneticModel stores a SphericalHarmonic that stores a SphericalEngine::coeff which stores references to the _gG/_hH vectors from the MagneticModel class.

So copying the MagneticModel, would copy these vectors, which would result in the copied MagneticModel's having references to the original classes vectors.

cffk commented 3 months ago

OK, and I suppose the move works in this case because the references to _gG and _hH are buried inside another object.

Can you add the same change for GravityModel to this PR?

ashbob999 commented 3 months ago

OK, and I suppose the move works in this case because the references to _gG and _hH are buried inside another object.

Yes, and moving them doesn't invalidate any references, so they stay valid.

Can you add the same change for GravityModel to this PR?

Sure.

cffk commented 3 months ago

@ashbob999 Thanks. This PR is merged now. FYI, I noticed that it included some trailing spaces. I always remove these so that the source is in a canonical format.