MenuConfig example mutating/setting class variables listed as Good
mpkocher opened this issue · 2 comments
mpkocher commented
I am surprised to see this example listed as "Good".
class MenuConfig:
"""A configuration for the Menu.
Attributes:
title: The title of the Menu.
body: The body of the Menu.
button_text: The text for the button label.
cancellable: Can it be cancelled?
"""
title: str
body: str
button_text: str
cancellable: bool = False
def create_menu(config: MenuConfig):
title = config.title
body = config.body
# ...
config = MenuConfig
config.title = "My delicious menu"
config.body = "A description of the various items on the menu"
config.button_text = "Order now!"
# The instance attribute overrides the default class attribute.
config.cancellable = True
create_menu(config)I think there might be some misunderstand of how classes work in Python or there's perhaps a typo yielding a confusion on instance vs class.
Any key:type annotation on a class gets added to __annotations__. If the key:type has a default then it will be set as the default class variable. Any setter on the class will set any value (e.g., config.dragons = 34). I believe this practice should be discouraged.
Here's an example:
In [13]: class Record:
...: alpha:int
...: beta:int
...: gamma:int = 1234
...:
...:
In [14]: r = Record
In [15]: r.alpha
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-15-b43492ebc57e> in <module>()
----> 1 r.alpha
AttributeError: type object 'Record' has no attribute 'alpha'
In [16]: r.__annotations__
Out[16]: {'alpha': int, 'beta': int, 'gamma': int}
In [17]: r.gamma
Out[17]: 1234
In [18]: r.dragons = 27
In [19]: r.more_dragons = 48
I would suggest that this Menu example should use an instance, not class, or the example should be marked as "Bad" in the text.