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

remove redundant code #2284

Closed
wants to merge 1 commit into from
Closed

Conversation

P4VEMENT
Copy link

@P4VEMENT P4VEMENT commented Sep 7, 2024

Remove redundant assignments of totalCostLimit and countLimit to memory storage properties since the config setter already did it.

@onevcat
Copy link
Owner

onevcat commented Sep 8, 2024

@P4VEMENT Thanks for this.

However, if I recall correctly, the didSet of a stored property isn’t triggered when it’s assigned within the initializer. Could you confirm if this change is necessary and ensure it won’t alter the current behavior?

Or maybe we can have a better way to improve this, for example extract the duplicated code into a helper method, etc.

@P4VEMENT
Copy link
Author

P4VEMENT commented Sep 8, 2024

As a beginner in the Swift language, thank you for the reminder. The official documentation states that "The willSet and didSet observers of superclass properties are called when a property is set in a subclass initializer, after the superclass initializer has been called. They aren’t called while a class is setting its own properties, before the superclass initializer has been called." I believe the current code is already sufficiently concise, so I will close this pull request.

@P4VEMENT P4VEMENT closed this Sep 8, 2024
@P4VEMENT P4VEMENT deleted the pavement/dev branch September 8, 2024 05:36
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.

2 participants