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

add makeHorizontalCoordinates(with geographicCoordinates: GeographicCoordinates) to CelestialBody Protocol #98

Closed
eheitfield opened this issue Sep 26, 2020 · 4 comments

Comments

@eheitfield
Copy link

eheitfield commented Sep 26, 2020

A very small API suggestion...

The Sun object includes the function makeHorizontalCoordinates(with:)which is convenient for folks wanting to plot the location of the sun in the sky. The CelestialBody protocol already has all the information one needs to compute rectangular coordinates (RA and HA), so why not add a default implementation of the function to that protocol so all objects (not just the Sun) get access to it?

While I'm here, I'd just like to say that this library is AWESOME! It's already saved me tons of time and has taught me a lot about celestial coordinates as well. I'm looking forward to using it in my next app. Thanks!!!

@onekiloparsec
Copy link
Owner

onekiloparsec commented Sep 27, 2020

Hi. Thanks for your message. Indeed, this could be added.

I've spent so much time trying to factor out everything, and play with this powerful Swift pattern which is protocol-oriented programming. But sometimes, I missed a few details in the global view.

I am glad it helped you in different ways. Don't hesitate to support it on patreon, it if you wish. ;-)

@onekiloparsec
Copy link
Owner

onekiloparsec commented Oct 8, 2020

I am currently moving the func makeHorizontalCoordinates(with geographicCoordinates: GeographicCoordinates) -> HorizontalCoordinates function from the Sun class to the CelestialBody protocol.

The little change is that it now uses self.equatorialCoordinates and not self.apparentEquatorialCoordinates as it was in the Sun class.

Hence to be consistent, I kept the method in the Sun class, with the existing implementation, but renaming it more correctly makeApparentHorizontalCoordinates(with geographicCoordinates: GeographicCoordinates) -> HorizontalCoordinates

Consequently: the Sun class has hence both methods: the standard one from the CelestialBodyprotocol, and the "apparent" one as a specific implementation.

@onekiloparsec
Copy link
Owner

I'll wait for closing #100 and #101 before making a patch release.

@onekiloparsec
Copy link
Owner

Already done.

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

No branches or pull requests

2 participants