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

Let child classes change default values of parent properties #473

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

t-kalinowski
Copy link
Member

closes #467

@t-kalinowski
Copy link
Member Author

This is tricky when a parent class has a custom constructor, where the default value is essentially unknown, and implicitly different from prop$default.

I.e., this test:

test_that("can use `...` in parent constructor", {

@lawremi, let me know if you have any thoughts about how to resolve this.

@t-kalinowski t-kalinowski marked this pull request as draft October 21, 2024 16:40
@lawremi
Copy link
Collaborator

lawremi commented Oct 25, 2024

Is my understanding correct that initialization of properties with default values only happens through the constructor? If so, then it seems tricky to implement a custom constructor, as it will need to support overriding of defaults.

One solution is the prototype idea, where the object starts with its default values. The constructor can always set them a second time, but just making sure they are set initially really simplifies the constructor contract.

Or, we just tell the user that custom constructors is an expert-level feature. Personally, I would avoid the type of constructor in the cited test and just leave the conveniences to a separate, user-facing constructor. Leaving the actual constructor to low-level property setting is a much more cohesive approach. In the core of Bioc, we rarely if ever overrode initialize(), which is the equivalent of a custom constructor, for these reasons.

@lawremi
Copy link
Collaborator

lawremi commented Nov 21, 2024

Would be good to revisit this, as it keeps biting me. The test that fails should not be relevant, since the subclass is not actually overriding any properties. Obviously, overriding a property (like inheritance itself) is strong coupling, so if the parent starts doing weird things in the constructor, things are expected to break.

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

Successfully merging this pull request may close these issues.

Default value introduced in subclass does not propagate to constructor
2 participants