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

refactor(subject): Subject.stream now returns a read-only Stream #699

Merged
merged 11 commits into from
Nov 16, 2022

Conversation

hoc081098
Copy link
Collaborator

@hoc081098 hoc081098 commented Nov 16, 2022

  • Streams returned by Subjects are read-only streams, ie. they don't support adding events (Previous, Subject.stream is identical to Subject).
  • Change return type of ReplaySubject<T>.stream to ReplayStream<T>

@hoc081098 hoc081098 added the enhancement New feature or request label Nov 16, 2022
@hoc081098 hoc081098 self-assigned this Nov 16, 2022
@hoc081098 hoc081098 changed the title test(subject): add Subject.stream getter tests refactor(subject): Subject.streams now return read-only streams Nov 16, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 16, 2022

Codecov Report

Merging #699 (b1b7030) into master (1a8fcef) will decrease coverage by 0.59%.
The diff coverage is 56.86%.

@@            Coverage Diff             @@
##           master     #699      +/-   ##
==========================================
- Coverage   93.74%   93.14%   -0.60%     
==========================================
  Files          77       77              
  Lines        2334     2378      +44     
==========================================
+ Hits         2188     2215      +27     
- Misses        146      163      +17     

@hoc081098 hoc081098 changed the title refactor(subject): Subject.streams now return read-only streams refactor(subject): Subject.streams now return read-only Streams Nov 16, 2022
@hoc081098 hoc081098 changed the title refactor(subject): Subject.streams now return read-only Streams refactor(subject): Subject.stream now returns a read-only Stream Nov 16, 2022
@hoc081098 hoc081098 merged commit e1f6cd9 into ReactiveX:master Nov 16, 2022
@hoc081098 hoc081098 deleted the rework_subject_stream branch November 16, 2022 07:41
@ryanheise
Copy link

I would prefer to leave this sort of thing the responsibility of static type checking. Adding defensive programming here increases the binary size and slightly adds to the computation time, whereas static type checking serves the same purpose but costs nothing at runtime.

What I mean regarding the static typing is that the return type of Subject.stream is already ValueStream<T> and so it already restricts the visible interface to the intended one. If a client does try to override the static types at runtime by casting that return type to some undocumented implementation type, it is using undocumented behaviour in which case the library is no longer responsible for perils the client suffers. There is no need to add executable code to defend against clients that try to bypass the declared static types.

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

Successfully merging this pull request may close these issues.

None yet

3 participants