Skip to content

MoonriseMoonset duplicates lunarPosition's logic instead of reusing it #53

Description

@philoserf

MoonriseMoonset duplicates lunarPosition's logic instead of reusing it

Severity: medium
Location: lunar.go:180-181, lunar.go:188-189

Description

lunarPosition (lunar.go:132-135) exists specifically to compute a Moon's
equatorial coordinates from an instant:

func lunarPosition(t time.Time) equatorial {
	ec := lunarEclipticPosition(t)
	return eclipticToEquatorial(t, ec.lon, ec.lat)
}

MoonriseMoonset needs exactly this, twice, but reimplements it inline
instead of calling it:

ec0 := lunarEclipticPosition(d)
eq0 := eclipticToEquatorial(d, ec0.lon, ec0.lat)
...
ec := lunarEclipticPosition(cur)
eq := eclipticToEquatorial(cur, ec.lon, ec.lat)

Because of this, lunarPosition is never called from production code —
only from lunar_test.go. This is pure duplication (not an intentional
algorithmic divergence, unlike solarPosition vs. the NOAA-method solar
declination path, which is deliberately documented as asymmetric). If the
nutation/obliquity handling in eclipticToEquatorial ever changes, there
are now two call sites in the same file that must be kept in sync instead
of one.

Suggested fix

Replace both inline sequences in MoonriseMoonset with calls to
lunarPosition:

eq0 := lunarPosition(d)
prevAlt := equatorialToHorizontal(d, obs, eq0).alt
...
for i := 1; i <= scanMinutes; i++ {
	cur := d.Add(time.Duration(i) * time.Minute)
	eq := lunarPosition(cur)
	hz := equatorialToHorizontal(cur, obs, eq)
	...
}

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions