I notice a bit of a weird syntax in an example in the mentoring notes for space-age. It looks like some auto-complete in an IDE went wrong
: Sign in to GitHub · GitHub
For example:
assert name: str.startswith("on_"), name: str
should read
assert name.startswith("on_"), name
Of course the same applies to the next line.
I’d also replace old-style new-style classes (inheriting from object), so that a quick copy-paste would not confuse users.
There’s an extra ) and a logic error in the same second example: round(self.years * PLANET_RATIOS[planet]), 2) → round(self.years / PLANET_RATIOS[planet], 2).
Personally, I’d also change the following:
- I see no good reason, in the context of the exercise, to define
yearsas a property - I’d rather move the computation from the
__getattr__()method to a separate method, and return apartial()on that method. Arguablypartialwould add an extra concept to introduce so maybe using a lambda is still better, although if we already get into attribute resolution and descriptors and whatnot,partial()is benign.
So with all of the above applied, the second example would look something like:
class SpaceAge:
def __init__(self, seconds):
self.seconds = seconds
def _age_on(self, planet: str) -> float:
result: float = round(self.seconds / EARTH_SECONDS / PLANET_RATIOS[planet], 2)
return result
def __getattr__(self, name: str):
assert name.startswith("on_"), name
planet = name.removeprefix("on_")
return partial(self._age_on, planet)
The last itemized list should not be a code block, either ![]()
There are some other options for reducing the code duplication in this exercise, but I don’t think any of these is a valid improvement:
- Using a
setattrrun in a top-level loop at import time poking in methods for each of the planets. Arguably faster since you don’t pay the price for lookups for each call, but it’s horrible. - Defining each
on_*method withpartialmethod. Not much of a saver in terms of DRY.
Given this is worth the time for review and so on, I’d be happy to create a PR. Please let me know if I should simply fix the syntax and logic, or implement one or more of the “extra” proposed changes.