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

Store segment flag on CO2 instance, not models #158

Conversation

EvanHahn
Copy link
Contributor

No user impact is expected from this change.

Background: you can ask CO2 to return segmented results by setting the results option to "segment". The CO2 instance stores this flag for later.

This changes where that flag is stored. Previously, it was stored as a public property of each model. This was unused by the model and is an unnecessary mutation.

Now, the property is stored on the CO2 instance. I believe this is easier to follow. It should also make it possible to turn the models from stateful classes into stateless objects, simplifying the code (an idea that was discussed years ago).

This new flag is explicitly marked private with the @private JSDoc tag and implicitly marked private by prefixing its name with an underscore; a common JS convention. (I considered using private class fields instead, but browser support is new enough that this seemed risky.)

No user impact is expected from this change.

Background: you can ask `CO2` to return segmented results by setting the
`results` option to `"segment"`. The `CO2` instance stores this flag for
later.

This changes where that flag is stored. Previously, it was stored as a
public property of each model. This was unused by the model and is an
unnecessary mutation.

Now, the property is stored on the `CO2` instance. I believe this is
easier to follow. It should also make it possible to turn the models
from stateful classes into stateless objects, simplifying the code
(an idea that was discussed [years ago][0]).

This new flag is explicitly marked private with the [`@private` JSDoc
tag][0] and implicitly marked private by prefixing its name with an
underscore; a common JS convention. (I considered using [private class
fields][1] instead, but browser support is new enough that this seemed
risky.)

[0]: thegreenwebfoundation#12 (comment)
[1]: https://jsdoc.app/tags-private.html
[2]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields
@fershad fershad merged commit 51e9d5e into thegreenwebfoundation:main Jul 21, 2023
3 checks passed
@fershad
Copy link
Contributor

fershad commented Jul 21, 2023

This change will be released with the next update for data, early August 2023.

@fershad fershad added this to the 0.13.x - Patch milestone Jul 21, 2023
@EvanHahn EvanHahn deleted the segment-is-a-property-of-co2-not-models branch July 21, 2023 03:46
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.

None yet

2 participants