Skip to content

Conversation

Sireikis
Copy link

@Sireikis Sireikis commented Sep 22, 2025

Disclaimer

This is an exercise in refactoring as defined by Martin Fowler--changing the internal structure of a program without changing behavior. No tests have been added or changed. No public API's should have been changed. I took care to make sure all tests pass and code compiles before every commit. Changes primarily consist of making code style more consistent and, as mentioned before, transforming the internal structure of code without altering behavior. Whether or not behavior has actually been changed however, depends entirely on the strength of the tests.

I'm aware refactoring is only really useful for code that changes, but since I did this for fun I may as well share my work, it may end up being useful.

Goal

The goal of this refactor was primarily to impose a more consistent style across SunKit and to make the Sun struct more readable. Care was taken to not alter any public APIs.

Note that there are files that have not been refactored: Angle, DMS, HMS, SunElevationEvents, and Utils.

Changes

-Style was made consistent by normalizing vertical and horizontal spacing. Spaces between types, functions, and operators were added or removed depending on context. Function names were given a consistent naming scheme when possible. Generally these changes should follow the swift language doc guidelines. Redundant comments (using Martin Fowler's definition) were removed when the code was self explanatory.
-Methods that are extensions on types were moved to appropriately named folders following the + naming convention.
-Mutations to EclipticCoordinates, EquatorialCoordinates, and HorizonCoordinates were encapsulated by the SunCoordinates struct.

Potential Future work

-There is more work to be done in Sun, such as encapsulating the Day Events and Month Events logic. That could be done in future PRs depending on the reception to this one.
-There is more refactoring to do in the various Coordinates structs, particularly the conversion code and the model mutation in EquatorialCoordinates. However, altering the way the EquatorialCoordinates mutates the model would require changes to the public API.
-Refactor Angle, DMS, HMS, SunElevationEvents, and Utils using the same process as in this PR.

Ignas Sireikis added 30 commits August 28, 2025 15:34
Ignas Sireikis added 27 commits September 19, 2025 17:15
…bject. Now using the setCoordinates method on SunCoordinates.
@seldon1000 seldon1000 added the enhancement New feature or request label Sep 24, 2025
@seldon1000
Copy link
Member

Hey, thanks for your contribution. Looks good to me, and I think that a cleanup was much needed. The other maintainer will review the changes too and possibly allow a merge.
As for future breaking changes in the public API, SunKit has currently settled and needs little to no maintenance. Yet, proposals will be very welcome.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants