Address coverage and issues #622
Conversation
WalkthroughWalkthroughThe changes encompass renaming adjustments and modifications to member variables across multiple files in the codebase. The key focus is on standardizing naming conventions, transitioning from CamelCase to lowercase with underscores for structures and types. Member variables gain the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (30)
- src/examples/rpp/doxygen/group_by.cpp (1 hunks)
- src/examples/rpp/sfml/snake/canvas.cpp (1 hunks)
- src/examples/rpp/sfml/snake/canvas.hpp (1 hunks)
- src/examples/rpp/sfml/snake/main.cpp (2 hunks)
- src/examples/rpp/sfml/snake/snake.cpp (3 hunks)
- src/examples/rpp/sfml/snake/utils.hpp (1 hunks)
- src/examples/rppgrpc/doxygen/server_reactor.cpp (1 hunks)
- src/extensions/rppgrpc/rppgrpc/details/base.hpp (4 hunks)
- src/rpp/rpp/defs.hpp (1 hunks)
- src/rpp/rpp/disposables/callback_disposable.hpp (1 hunks)
- src/rpp/rpp/disposables/details/container.hpp (1 hunks)
- src/rpp/rpp/observables/connectable_observable.hpp (1 hunks)
- src/rpp/rpp/observers/dynamic_observer.hpp (1 hunks)
- src/rpp/rpp/observers/mock_observer.hpp (3 hunks)
- src/rpp/rpp/operators/concat.hpp (1 hunks)
- src/rpp/rpp/operators/delay.hpp (1 hunks)
- src/rpp/rpp/operators/timeout.hpp (2 hunks)
- src/rpp/rpp/operators/window.hpp (1 hunks)
- src/rpp/rpp/operators/with_latest_from.hpp (2 hunks)
- src/rpp/rpp/schedulers/computational.hpp (1 hunks)
- src/rpp/rpp/schedulers/current_thread.hpp (1 hunks)
- src/rpp/rpp/schedulers/details/queue.hpp (1 hunks)
- src/rpp/rpp/schedulers/test_scheduler.hpp (1 hunks)
- src/rpp/rpp/schedulers/thread_pool.hpp (2 hunks)
- src/rpp/rpp/utils/utils.hpp (4 hunks)
- src/tests/rpp/test_debounce.cpp (2 hunks)
- src/tests/rpp/test_scheduler.cpp (2 hunks)
- src/tests/rppqt/test_from_signal.cpp (4 hunks)
- src/tests/utils/copy_count_tracker.hpp (2 hunks)
- src/tests/utils/rpp_trompeloil.hpp (1 hunks)
Files skipped from review due to trivial changes (19)
- src/examples/rpp/doxygen/group_by.cpp
- src/examples/rpp/sfml/snake/main.cpp
- src/examples/rpp/sfml/snake/snake.cpp
- src/extensions/rppgrpc/rppgrpc/details/base.hpp
- src/rpp/rpp/defs.hpp
- src/rpp/rpp/disposables/details/container.hpp
- src/rpp/rpp/observables/connectable_observable.hpp
- src/rpp/rpp/observers/mock_observer.hpp
- src/rpp/rpp/operators/window.hpp
- src/rpp/rpp/operators/with_latest_from.hpp
- src/rpp/rpp/schedulers/computational.hpp
- src/rpp/rpp/schedulers/current_thread.hpp
- src/rpp/rpp/schedulers/details/queue.hpp
- src/rpp/rpp/schedulers/thread_pool.hpp
- src/rpp/rpp/utils/utils.hpp
- src/tests/rpp/test_scheduler.cpp
- src/tests/rppqt/test_from_signal.cpp
- src/tests/utils/copy_count_tracker.hpp
- src/tests/utils/rpp_trompeloil.hpp
Additional comments not posted (31)
src/examples/rpp/sfml/snake/canvas.hpp (1)
7-7: Verify the consistency of type name change.Ensure that the type name change from
Coordinatestocoordinatesis consistent across the codebase to avoid any discrepancies.Run the following script to verify the consistency:
Verification successful
Type name change is consistent across the codebase.
The type name change from
Coordinatestocoordinateshas been consistently applied across the codebase, with no occurrences of the old type name found.
- Files with
coordinates:
src/examples/rpp/sfml/snake/utils.hppsrc/examples/rpp/sfml/snake/snake.cppsrc/examples/rpp/sfml/snake/canvas.cppsrc/examples/rpp/sfml/snake/canvas.hppScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of type name change from `Coordinates` to `coordinates`. # Test: Search for the old type name. Expect: No occurrences of `Coordinates`. rg --type cpp --type h $'Coordinates' # Test: Search for the new type name. Expect: Occurrences of `coordinates`. rg --type cpp --type h $'coordinates'Length of output: 1966
src/examples/rpp/sfml/snake/canvas.cpp (2)
Line range hint
9-16: LGTM!The implementation of the function remains unchanged and appears to be correct.
9-9: Verify the consistency of type name change.Ensure that the type name change from
Coordinatestocoordinatesis consistent across the codebase to avoid any discrepancies.Run the following script to verify the consistency:
Verification successful
Type name change verified successfully.
The type name change from
Coordinatestocoordinateshas been consistently applied across the codebase. No occurrences of the old type name were found, confirming the successful update.
- Instances of
coordinatesfound in relevant files, ensuring consistency.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of type name change from `Coordinates` to `coordinates`. # Test: Search for the old type name. Expect: No occurrences of `Coordinates`. rg --type cpp --type h $'Coordinates' # Test: Search for the new type name. Expect: Occurrences of `coordinates`. rg --type cpp --type h $'coordinates'Length of output: 1966
src/examples/rpp/sfml/snake/utils.hpp (7)
19-20: LGTM!The operator overloads for equality and inequality appear to be correct.
37-38: LGTM!The function implementation appears to be correct and consistent with the changes in naming conventions.
14-20: Verify the consistency of structure name change.Ensure that the structure name change from
Coordinatestocoordinatesis consistent across the codebase to avoid any discrepancies.Run the following script to verify the consistency:
Verification successful
Structure name change is consistent.
The structure name change from
Coordinatestocoordinateshas been applied consistently across the codebase, as no occurrences of the old name were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of structure name change from `Coordinates` to `coordinates`. # Test: Search for the old structure name. Expect: No occurrences of `Coordinates`. rg --type cpp --type h $'struct Coordinates' # Test: Search for the new structure name. Expect: Occurrences of `coordinates`. rg --type cpp --type h $'struct coordinates'Length of output: 148
23-23: Verify the consistency of type alias change.Ensure that the type alias change from
Coordinatestocoordinatesis consistent across the codebase to avoid any discrepancies.Run the following script to verify the consistency:
Verification successful
Type alias change is consistent across the codebase.
The type alias change from
Coordinatestocoordinateshas been applied consistently throughout the codebase, with no remaining instances of the old alias.
- The new alias
using Direction = coordinates;is correctly implemented insrc/examples/rpp/sfml/snake/utils.hpp.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of type alias change from `Coordinates` to `coordinates`. # Test: Search for the old type alias. Expect: No occurrences of `Coordinates`. rg --type cpp --type h $'using Direction = Coordinates' # Test: Search for the new type alias. Expect: Occurrences of `coordinates`. rg --type cpp --type h $'using Direction = coordinates'Length of output: 182
33-33: Verify the consistency of type alias change.Ensure that the type alias change from
PresentEventtopresent_eventis consistent across the codebase to avoid any discrepancies.Run the following script to verify the consistency:
Verification successful
Type alias change is consistent across the codebase.
The type alias change from
PresentEventtopresent_eventis consistently applied, with no occurrences of the old alias found. The new alias is correctly used in the expected location.
src/examples/rpp/sfml/snake/utils.hpp:using CustomEvent = std::variant<present_event, sf::Event>;Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of type alias change from `PresentEvent` to `present_event`. # Test: Search for the old type alias. Expect: No occurrences of `PresentEvent`. rg --type cpp --type h $'using CustomEvent = std::variant<PresentEvent, sf::Event>' # Test: Search for the new type alias. Expect: Occurrences of `present_event`. rg --type cpp --type h $'using CustomEvent = std::variant<present_event, sf::Event>'Length of output: 268
28-28: Verify the consistency of structure name change.Ensure that the structure name change from
PresentEventtopresent_eventis consistent across the codebase to avoid any discrepancies.Run the following script to verify the consistency:
24-24: Verify the consistency of type alias change.Ensure that the type alias change from
Coordinatestocoordinatesis consistent across the codebase to avoid any discrepancies.Run the following script to verify the consistency:
Verification successful
The type alias change is consistent across the codebase.
The change from
Coordinatestocoordinateshas been applied consistently throughout the codebase, and thecoordinatesstructure is defined and used correctly in the relevant files.
- The
coordinatesstructure is defined insrc/examples/rpp/sfml/snake/utils.hpp.- The
coordinatesstructure is used incanvas.cpp,snake.cpp, andcanvas.hpp.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of type alias change from `Coordinates` to `coordinates`. # Test: Search for the old type alias. Expect: No occurrences of `Coordinates`. rg --type cpp --type h $'using SnakeBody = std::vector<Coordinates>' # Test: Search for the new type alias. Expect: Occurrences of `coordinates`. rg --type cpp --type h $'using SnakeBody = std::vector<coordinates>'Length of output: 221
Script:
#!/bin/bash # Description: Verify the definition and usage of `coordinates` across the codebase. # Search for the definition of `coordinates`. rg --type cpp --type h 'coordinates' # Search for the usage of `coordinates` in other contexts. rg --type cpp --type h '\bcoordinates\b'Length of output: 3865
src/rpp/rpp/disposables/callback_disposable.hpp (1)
43-43: LGTM! TheRPP_NO_UNIQUE_ADDRESSattribute optimizes memory usage.The addition of the
RPP_NO_UNIQUE_ADDRESSattribute is a good optimization for small objects.src/examples/rppgrpc/doxygen/server_reactor.cpp (1)
12-12: LGTM! But verify the class references in the codebase.The renaming of the class
Servertoserveris approved.However, ensure that all references to the class
Serverare updated toserverin the codebase.Run the following script to verify the class references:
Verification successful
Class references updated successfully.
The renaming of the class from
Servertoserverhas been applied consistently across the codebase. No references to the old class nameServerwere found.
- All occurrences of the class
serverare correctly updated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the class `Server` are updated to `server`. # Test: Search for the class references. Expect: Only occurrences of the new class name. rg --type cpp -A 5 $'class server'Length of output: 2686
src/rpp/rpp/schedulers/test_scheduler.hpp (10)
15-15: Encapsulation oftest_schedulerclass withinrpp::schedulersnamespace.The encapsulation of the
test_schedulerclass within therpp::schedulersnamespace improves organization and clarity.
19-19: Final class declaration fortest_scheduler.Declaring the
test_schedulerclass asfinalis a good practice to prevent further inheritance.
22-22: Addition ofinlinekeyword tos_current_time.The addition of the
inlinekeyword to thes_current_timemember promotes better linkage across translation units.
24-24: Reordering ofworker_strategyclass declaration.Reordering the
worker_strategyclass declaration after thetest_schedulerclass maintains public access and improves logical grouping.
26-26: Reordering ofstatestruct declaration.Reordering the
statestruct declaration after theworker_strategyclass reflects a clearer hierarchy and relationship between these components.
30-38: Improvement inschedulemethod.The
schedulemethod now ensures that it checks for disposal status before proceeding with scheduling tasks.
40-60: Refinement ofdrainmethod.The
drainmethod has been refined to enhance its logic, particularly in how it handles the queue and time points, ensuring that it only processes items when appropriate.
65-70: Base dispose implementation and member variables instatestruct.The
base_dispose_implmethod and member variables in thestatestruct are well-defined and maintain consistency.
72-99: Enhancements inworker_strategyclass.The
worker_strategyclass has been enhanced to include a check for the locked state before scheduling, ensuring that disposed states are handled correctly. This change enhances the robustness of the scheduling mechanism.
101-121: Overall improvements intest_schedulerclass.The overall improvements in the
test_schedulerclass, including method implementations and member variables, enhance the encapsulation, readability, and maintainability of the code while preserving the original functionality.src/rpp/rpp/observers/dynamic_observer.hpp (1)
87-87: LGTM! TheRPP_NO_UNIQUE_ADDRESSattribute optimizes memory layout.Adding the
RPP_NO_UNIQUE_ADDRESSattribute can optimize memory usage by allowing the compiler to place the member variable more efficiently.src/tests/rpp/test_debounce.cpp (3)
25-25: LGTM! Explicitly referencingtest_schedulerfromrpp::schedulersenhances clarity.This change ensures that the correct type is utilized and improves the readability of the code.
26-26: LGTM! Explicitly referencings_current_timefromrpp::schedulers::test_schedulerensures consistency.This change reduces potential ambiguity about which
s_current_timeis being referenced and improves the maintainability of the code.
126-126: LGTM! Explicitly usingrpp::schedulers::test_schedulerimproves readability.This change ensures that the correct type is utilized and enhances the readability of the code.
src/rpp/rpp/operators/concat.hpp (1)
Line range hint
28-32: LGTM! ChangingConcatStageto a scoped enumeration enhances type safety.This change prevents name clashes by requiring the scope to be specified when accessing the enumeration values. The underlying type remains
uint8_t, ensuring that the memory footprint is unchanged.src/rpp/rpp/operators/delay.hpp (2)
55-55: LGTM! Verify the impact ofRPP_NO_UNIQUE_ADDRESSonobserver.The change optimizes memory usage by allowing the compiler to eliminate the storage of the member if it is the only member in the struct.
Ensure that this change does not introduce any unexpected behavior in the codebase.
Run the following script to verify the usage of
observer:
56-56: LGTM! Verify the impact ofRPP_NO_UNIQUE_ADDRESSonworker.The change optimizes memory usage by allowing the compiler to eliminate the storage of the member if it is the only member in the struct.
Ensure that this change does not introduce any unexpected behavior in the codebase.
Run the following script to verify the usage of
worker:Verification successful
No issues found with
RPP_NO_UNIQUE_ADDRESSusage onworker.The change optimizes memory usage without introducing any unexpected behavior in the codebase. The extensive use of
workerin tests and scheduler implementations does not reveal any problems related to this change.
- The
workeris used in various contexts, including scheduling tasks and managing disposables, without any issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `worker` in the codebase. # Test: Search for the usage of `worker`. Expect: No unexpected behavior. rg --type cpp -A 5 $'worker'Length of output: 89634
src/rpp/rpp/operators/timeout.hpp (2)
28-28: LGTM! Verify the impact ofRPP_NO_UNIQUE_ADDRESSonobserver.The change optimizes memory usage by allowing the compiler to eliminate the storage of the member if it is the only member in the struct.
Ensure that this change does not introduce any unexpected behavior in the codebase.
Run the following script to verify the usage of
observer:
48-48: LGTM! Verify the impact ofRPP_NO_UNIQUE_ADDRESSonm_fallback.The change optimizes memory usage by allowing the compiler to eliminate the storage of the member if it is the only member in the struct.
Ensure that this change does not introduce any unexpected behavior in the codebase.
Run the following script to verify the usage of
m_fallback:Verification successful
Change Verified:
RPP_NO_UNIQUE_ADDRESSonm_fallbackis safe.The
RPP_NO_UNIQUE_ADDRESSattribute optimizes memory usage without affecting the logic of the code. The usage ofm_fallbackin the constructor and getter method confirms its functionality remains intact.
- The attribute is applied to
m_fallbackinsrc/rpp/rpp/operators/timeout.hpp.m_fallbackis used in the constructor and a getter method, ensuring its role in the class is preserved.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `m_fallback` in the codebase. # Test: Search for the usage of `m_fallback`. Expect: No unexpected behavior. rg --type cpp -A 5 $'m_fallback'Length of output: 1348
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- src/rpp/rpp/operators/window.hpp (2 hunks)
- src/rpp/rpp/operators/with_latest_from.hpp (1 hunks)
- src/tests/rpp/test_delay.cpp (5 hunks)
- src/tests/rpp/test_interval.cpp (2 hunks)
- src/tests/rpp/test_retry.cpp (3 hunks)
- src/tests/rpp/test_scheduler.cpp (3 hunks)
- src/tests/rpp/test_throttle.cpp (2 hunks)
- src/tests/rpp/test_timeout.cpp (2 hunks)
- src/tests/rpp/test_timer.cpp (2 hunks)
Files skipped from review due to trivial changes (5)
- src/rpp/rpp/operators/with_latest_from.hpp
- src/tests/rpp/test_delay.cpp
- src/tests/rpp/test_throttle.cpp
- src/tests/rpp/test_timeout.cpp
- src/tests/rpp/test_timer.cpp
Files skipped from review as they are similar to previous changes (2)
- src/rpp/rpp/operators/window.hpp
- src/tests/rpp/test_scheduler.cpp
Additional comments not posted (5)
src/tests/rpp/test_interval.cpp (2)
28-28: LGTM! Namespace update fortest_schedulerimproves clarity.The change to use the fully qualified name
rpp::schedulers::test_schedulerensures that the correct namespace is specified, enhancing clarity and preventing ambiguity.
35-35: LGTM! Namespace update forworker_strategy::now()improves clarity.The change to use the fully qualified name
rpp::schedulers::test_scheduler::worker_strategy::now()ensures that the correct namespace is specified, enhancing clarity and preventing ambiguity.src/tests/rpp/test_retry.cpp (3)
14-17: LGTM! New includes for multi-threading tests.The added includes for
rpp/operators/subscribe_on.hppandrpp/schedulers/new_thread.hppare necessary for the new test cases that involve multi-threading with theretryoperator.
63-71: LGTM! New test case for multi-threading withretryoperator.The added test case verifies the behavior of the
retryoperator when invoked from another thread, ensuring that it handles multi-threading scenarios correctly.
137-152: LGTM! New test case for exception handling withretryoperator.The added test case verifies the behavior of the
retryoperator when the observable throws an exception, ensuring that it handles exceptions correctly.
BENCHMARK RESULTS (AUTOGENERATED)
|
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 300.17 ns | 2.16 ns | 2.16 ns | 1.00 |
| Subscribe empty callbacks to empty observable via pipe operator | 298.54 ns | 2.16 ns | 2.16 ns | 1.00 |
Sources
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 683.83 ns | 0.31 ns | 0.31 ns | 1.00 |
| from array of 1 - create + subscribe + current_thread | 1035.37 ns | 3.43 ns | 3.70 ns | 0.93 |
| concat_as_source of just(1 immediate) create + subscribe | 2180.40 ns | 123.34 ns | 105.08 ns | 1.17 |
| defer from array of 1 - defer + create + subscribe + immediate | 724.49 ns | 0.31 ns | 0.31 ns | 1.00 |
| interval - interval + take(3) + subscribe + immediate | 2143.04 ns | 59.23 ns | 59.33 ns | 1.00 |
| interval - interval + take(3) + subscribe + current_thread | 3019.81 ns | 32.40 ns | 32.40 ns | 1.00 |
| from array of 1 - create + as_blocking + subscribe + new_thread | 30948.97 ns | 28839.78 ns | 28073.41 ns | 1.03 |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 39125.18 ns | 51192.71 ns | 50189.17 ns | 1.02 |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 3414.91 ns | 135.85 ns | 126.09 ns | 1.08 |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 1085.06 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just+filter(true)+subscribe | 830.57 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just(1,2)+skip(1)+subscribe | 979.49 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 870.90 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just(1,2)+first()+subscribe | 1230.11 ns | 0.62 ns | 0.62 ns | 1.00 |
| immediate_just(1,2)+last()+subscribe | 899.65 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just+take_last(1)+subscribe | 1091.78 ns | 17.29 ns | 17.60 ns | 0.98 |
| immediate_just(1,2,3)+element_at(1)+subscribe | 834.58 ns | 0.31 ns | 0.31 ns | 1.00 |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate scheduler create worker + schedule | 263.86 ns | 2.16 ns | 2.16 ns | 1.00 |
| current_thread scheduler create worker + schedule | 364.64 ns | 5.56 ns | 5.86 ns | 0.95 |
| current_thread scheduler create worker + schedule + recursive schedule | 811.69 ns | 56.52 ns | 56.54 ns | 1.00 |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 845.12 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just+scan(10, std::plus)+subscribe | 880.64 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 2372.30 ns | 162.33 ns | 159.52 ns | 1.02 |
| immediate_just+buffer(2)+subscribe | 1535.66 ns | 13.59 ns | 13.58 ns | 1.00 |
| immediate_just+window(2)+subscribe + subscsribe inner | 2355.29 ns | 1108.84 ns | 1028.32 ns | 1.08 |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 829.23 ns | - | - | 0.00 |
| immediate_just+take_while(true)+subscribe | 847.14 ns | 0.31 ns | 0.31 ns | 1.00 |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 2030.08 ns | 0.31 ns | 0.31 ns | 1.00 |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 3413.50 ns | 175.97 ns | 179.31 ns | 0.98 |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 3651.41 ns | 171.04 ns | 185.48 ns | 0.92 |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 134.75 ns | 132.96 ns | 1.01 |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 3517.78 ns | 928.90 ns | 997.81 ns | 0.93 |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 2110.56 ns | 204.03 ns | 206.75 ns | 0.99 |
Subjects
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 34.58 ns | 30.86 ns | 14.72 ns | 2.10 |
| subscribe 100 observers to publish_subject | 200040.17 ns | 14905.36 ns | 15319.15 ns | 0.97 |
| 100 on_next to 100 observers to publish_subject | 32463.64 ns | 17139.90 ns | 18842.10 ns | 0.91 |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| basic sample | 1424.35 ns | 12.97 ns | 12.65 ns | 1.03 |
| basic sample with immediate scheduler | 1383.93 ns | 5.55 ns | 5.55 ns | 1.00 |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 937.75 ns | 0.31 ns | 0.31 ns | 1.00 |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 2042.20 ns | 1024.94 ns | 996.08 ns | 1.03 |
| create(on_error())+retry(1)+subscribe | 584.75 ns | 117.05 ns | 111.07 ns | 1.05 |
ci-macos
General
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 980.20 ns | 3.93 ns | 5.40 ns | 0.73 |
| Subscribe empty callbacks to empty observable via pipe operator | 977.58 ns | 3.93 ns | 4.62 ns | 0.85 |
Sources
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 1934.07 ns | 0.23 ns | 0.32 ns | 0.73 |
| from array of 1 - create + subscribe + current_thread | 2442.50 ns | 34.24 ns | 39.03 ns | 0.88 |
| concat_as_source of just(1 immediate) create + subscribe | 5407.46 ns | 335.65 ns | 420.96 ns | 0.80 |
| defer from array of 1 - defer + create + subscribe + immediate | 1975.92 ns | 0.23 ns | 0.28 ns | 0.85 |
| interval - interval + take(3) + subscribe + immediate | 4944.91 ns | 113.89 ns | 133.53 ns | 0.85 |
| interval - interval + take(3) + subscribe + current_thread | 6029.44 ns | 98.14 ns | 110.64 ns | 0.89 |
| from array of 1 - create + as_blocking + subscribe + new_thread | 81797.17 ns | 80619.58 ns | 134078.29 ns | 0.60 |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 85318.77 ns | 84408.00 ns | 151241.57 ns | 0.56 |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 8156.84 ns | 380.68 ns | 448.26 ns | 0.85 |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 2725.59 ns | 0.22 ns | 0.29 ns | 0.76 |
| immediate_just+filter(true)+subscribe | 2024.33 ns | 0.22 ns | 0.28 ns | 0.79 |
| immediate_just(1,2)+skip(1)+subscribe | 2621.91 ns | 0.22 ns | 0.29 ns | 0.77 |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 1988.13 ns | 0.45 ns | 0.60 ns | 0.75 |
| immediate_just(1,2)+first()+subscribe | 3023.43 ns | 0.22 ns | 0.27 ns | 0.82 |
| immediate_just(1,2)+last()+subscribe | 2288.00 ns | 0.22 ns | 0.33 ns | 0.67 |
| immediate_just+take_last(1)+subscribe | 2872.18 ns | 0.22 ns | 0.28 ns | 0.81 |
| immediate_just(1,2,3)+element_at(1)+subscribe | 2120.41 ns | 0.23 ns | 0.28 ns | 0.81 |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate scheduler create worker + schedule | 860.79 ns | 4.14 ns | 4.85 ns | 0.85 |
| current_thread scheduler create worker + schedule | 1200.19 ns | 39.89 ns | 42.73 ns | 0.93 |
| current_thread scheduler create worker + schedule + recursive schedule | 2008.95 ns | 207.14 ns | 242.41 ns | 0.85 |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 2024.53 ns | 4.23 ns | 5.17 ns | 0.82 |
| immediate_just+scan(10, std::plus)+subscribe | 2237.26 ns | 0.45 ns | 0.62 ns | 0.72 |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 5074.79 ns | 387.47 ns | 517.27 ns | 0.75 |
| immediate_just+buffer(2)+subscribe | 2386.73 ns | 63.04 ns | 81.41 ns | 0.77 |
| immediate_just+window(2)+subscribe + subscsribe inner | 5086.12 ns | 2300.69 ns | 3319.91 ns | 0.69 |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 2021.47 ns | - | - | 0.00 |
| immediate_just+take_while(true)+subscribe | 2029.79 ns | 0.22 ns | 0.34 ns | 0.65 |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 4826.18 ns | 4.78 ns | 6.16 ns | 0.78 |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 7418.18 ns | 440.99 ns | 528.61 ns | 0.83 |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 8017.82 ns | 441.61 ns | 516.77 ns | 0.85 |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 439.40 ns | 605.83 ns | 0.73 |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 7608.75 ns | 1816.48 ns | 2994.50 ns | 0.61 |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 4886.53 ns | 804.89 ns | 1137.75 ns | 0.71 |
Subjects
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 71.91 ns | 45.73 ns | 60.47 ns | 0.76 |
| subscribe 100 observers to publish_subject | 330138.00 ns | 38284.90 ns | 51052.45 ns | 0.75 |
| 100 on_next to 100 observers to publish_subject | 51363.39 ns | 17467.51 ns | 22372.85 ns | 0.78 |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| basic sample | 2772.72 ns | 79.54 ns | 85.42 ns | 0.93 |
| basic sample with immediate scheduler | 2758.38 ns | 18.73 ns | 21.80 ns | 0.86 |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 2329.59 ns | 0.23 ns | 0.33 ns | 0.69 |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 6326.77 ns | 4001.06 ns | 4848.54 ns | 0.83 |
| create(on_error())+retry(1)+subscribe | 1689.12 ns | 292.17 ns | 362.58 ns | 0.81 |
ci-ubuntu-clang
General
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 268.30 ns | 0.88 ns | 1.54 ns | 0.57 |
| Subscribe empty callbacks to empty observable via pipe operator | 271.23 ns | 0.88 ns | 1.54 ns | 0.57 |
Sources
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 563.13 ns | 0.32 ns | 0.34 ns | 0.92 |
| from array of 1 - create + subscribe + current_thread | 790.72 ns | 4.01 ns | 4.32 ns | 0.93 |
| concat_as_source of just(1 immediate) create + subscribe | 2328.27 ns | 135.82 ns | 134.92 ns | 1.01 |
| defer from array of 1 - defer + create + subscribe + immediate | 795.37 ns | 0.31 ns | 0.31 ns | 1.00 |
| interval - interval + take(3) + subscribe + immediate | 2206.65 ns | 58.31 ns | 58.31 ns | 1.00 |
| interval - interval + take(3) + subscribe + current_thread | 3140.17 ns | 30.88 ns | 30.86 ns | 1.00 |
| from array of 1 - create + as_blocking + subscribe + new_thread | 28495.16 ns | 28182.72 ns | 28005.07 ns | 1.01 |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 39316.23 ns | 36401.56 ns | 36459.93 ns | 1.00 |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 3683.85 ns | 159.42 ns | 156.82 ns | 1.02 |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 1177.55 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just+filter(true)+subscribe | 835.12 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just(1,2)+skip(1)+subscribe | 1078.33 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 868.78 ns | 0.62 ns | 0.62 ns | 1.00 |
| immediate_just(1,2)+first()+subscribe | 1375.55 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just(1,2)+last()+subscribe | 1008.43 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just+take_last(1)+subscribe | 1170.61 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just(1,2,3)+element_at(1)+subscribe | 855.06 ns | 0.31 ns | 0.31 ns | 1.00 |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate scheduler create worker + schedule | 278.99 ns | 0.88 ns | 1.54 ns | 0.57 |
| current_thread scheduler create worker + schedule | 388.84 ns | 4.02 ns | 4.79 ns | 0.84 |
| current_thread scheduler create worker + schedule + recursive schedule | 854.23 ns | 56.92 ns | 55.85 ns | 1.02 |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 840.25 ns | 0.31 ns | 0.31 ns | 1.00 |
| immediate_just+scan(10, std::plus)+subscribe | 969.25 ns | 0.32 ns | 0.31 ns | 1.05 |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 2245.07 ns | 137.39 ns | 144.46 ns | 0.95 |
| immediate_just+buffer(2)+subscribe | 1525.92 ns | 13.92 ns | 13.88 ns | 1.00 |
| immediate_just+window(2)+subscribe + subscsribe inner | 2416.46 ns | 928.56 ns | 936.43 ns | 0.99 |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 836.17 ns | - | - | 0.00 |
| immediate_just+take_while(true)+subscribe | 841.04 ns | 0.31 ns | 0.31 ns | 1.00 |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 1972.23 ns | 0.31 ns | 0.31 ns | 1.00 |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 3255.10 ns | 160.77 ns | 159.69 ns | 1.01 |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 3706.10 ns | 146.61 ns | 145.90 ns | 1.00 |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 143.04 ns | 144.26 ns | 0.99 |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 3386.34 ns | 856.44 ns | 845.49 ns | 1.01 |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 2227.60 ns | 201.74 ns | 200.63 ns | 1.01 |
Subjects
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 52.24 ns | 17.67 ns | 17.65 ns | 1.00 |
| subscribe 100 observers to publish_subject | 204017.80 ns | 15917.10 ns | 16378.32 ns | 0.97 |
| 100 on_next to 100 observers to publish_subject | 37887.46 ns | 17931.81 ns | 21234.39 ns | 0.84 |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| basic sample | 1311.18 ns | 11.42 ns | 11.72 ns | 0.97 |
| basic sample with immediate scheduler | 1281.44 ns | 5.86 ns | 6.17 ns | 0.95 |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 1002.25 ns | 0.31 ns | 0.31 ns | 1.00 |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 2172.29 ns | 1258.12 ns | 1215.57 ns | 1.04 |
| create(on_error())+retry(1)+subscribe | 643.59 ns | 146.47 ns | 145.97 ns | 1.00 |
ci-windows
General
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| Subscribe empty callbacks to empty observable | 561.76 ns | 4.94 ns | 4.69 ns | 1.05 |
| Subscribe empty callbacks to empty observable via pipe operator | 577.12 ns | 4.87 ns | 4.79 ns | 1.02 |
Sources
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| from array of 1 - create + subscribe + immediate | 1166.27 ns | 9.70 ns | 4.62 ns | 2.10 |
| from array of 1 - create + subscribe + current_thread | 1435.10 ns | 17.59 ns | 15.51 ns | 1.13 |
| concat_as_source of just(1 immediate) create + subscribe | 3692.23 ns | 192.64 ns | 172.56 ns | 1.12 |
| defer from array of 1 - defer + create + subscribe + immediate | 1185.96 ns | 9.41 ns | 4.94 ns | 1.91 |
| interval - interval + take(3) + subscribe + immediate | 3381.19 ns | 144.87 ns | 133.24 ns | 1.09 |
| interval - interval + take(3) + subscribe + current_thread | 3409.06 ns | 65.48 ns | 54.57 ns | 1.20 |
| from array of 1 - create + as_blocking + subscribe + new_thread | 122830.00 ns | 118400.00 ns | 109020.00 ns | 1.09 |
| from array of 1000 - create + as_blocking + subscribe + new_thread | 130533.33 ns | 133262.50 ns | 126250.00 ns | 1.06 |
| concat_as_source of just(1 immediate) and just(1,2 immediate)create + subscribe | 5317.59 ns | 211.57 ns | 206.73 ns | 1.02 |
Filtering Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take(1)+subscribe | 1826.35 ns | 24.97 ns | 12.87 ns | 1.94 |
| immediate_just+filter(true)+subscribe | 1315.63 ns | 24.06 ns | 12.37 ns | 1.95 |
| immediate_just(1,2)+skip(1)+subscribe | 1737.63 ns | 23.44 ns | 13.13 ns | 1.78 |
| immediate_just(1,1,2)+distinct_until_changed()+subscribe | 1593.61 ns | 26.26 ns | 15.94 ns | 1.65 |
| immediate_just(1,2)+first()+subscribe | 2345.01 ns | 23.74 ns | 12.94 ns | 1.83 |
| immediate_just(1,2)+last()+subscribe | 1801.11 ns | 24.69 ns | 13.80 ns | 1.79 |
| immediate_just+take_last(1)+subscribe | 2001.12 ns | 69.32 ns | 58.99 ns | 1.18 |
| immediate_just(1,2,3)+element_at(1)+subscribe | 1337.51 ns | 26.54 ns | 13.78 ns | 1.93 |
Schedulers
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate scheduler create worker + schedule | 472.54 ns | 5.87 ns | 6.17 ns | 0.95 |
| current_thread scheduler create worker + schedule | 644.22 ns | 13.82 ns | 14.07 ns | 0.98 |
| current_thread scheduler create worker + schedule + recursive schedule | 1087.14 ns | 103.03 ns | 104.67 ns | 0.98 |
Transforming Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+map(v*2)+subscribe | 1305.53 ns | 24.35 ns | 12.34 ns | 1.97 |
| immediate_just+scan(10, std::plus)+subscribe | 1403.79 ns | 26.51 ns | 21.58 ns | 1.23 |
| immediate_just+flat_map(immediate_just(v*2))+subscribe | 3870.48 ns | 204.19 ns | 203.81 ns | 1.00 |
| immediate_just+buffer(2)+subscribe | 2304.71 ns | 68.64 ns | 58.38 ns | 1.18 |
| immediate_just+window(2)+subscribe + subscsribe inner | 4015.03 ns | 1283.84 ns | 1322.69 ns | 0.97 |
Conditional Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+take_while(false)+subscribe | 1312.44 ns | 23.14 ns | 11.46 ns | 2.02 |
| immediate_just+take_while(true)+subscribe | 1323.33 ns | 24.06 ns | 12.37 ns | 1.94 |
Utility Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(1)+subscribe_on(immediate)+subscribe | 3443.63 ns | 11.10 ns | 7.40 ns | 1.50 |
Combining Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just(immediate_just(1), immediate_just(1)) + merge() + subscribe | 5326.56 ns | 220.27 ns | 225.94 ns | 0.97 |
| immediate_just(1) + merge_with(immediate_just(2)) + subscribe | 5444.98 ns | 226.72 ns | 214.63 ns | 1.06 |
| immediate_just(1) + with_latest_from(immediate_just(2)) + subscribe | - | 200.87 ns | 193.76 ns | 1.04 |
| immediate_just(immediate_just(1),immediate_just(1)) + switch_on_next() + subscribe | 5423.11 ns | 922.00 ns | 942.66 ns | 0.98 |
| immediate_just(1) + zip(immediate_just(2)) + subscribe | 3539.82 ns | 515.05 ns | 519.55 ns | 0.99 |
Subjects
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| publish_subject with 1 observer - on_next | 37.39 ns | 20.36 ns | 19.79 ns | 1.03 |
| subscribe 100 observers to publish_subject | 304275.00 ns | 26852.63 ns | 27657.14 ns | 0.97 |
| 100 on_next to 100 observers to publish_subject | 51755.00 ns | 32683.87 ns | 38686.21 ns | 0.84 |
Scenarios
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| basic sample | 1875.04 ns | 101.93 ns | 58.62 ns | 1.74 |
| basic sample with immediate scheduler | 1886.75 ns | 72.49 ns | 36.72 ns | 1.97 |
Aggregating Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| immediate_just+reduce(10, std::plus)+subscribe | 1449.08 ns | 24.66 ns | 19.97 ns | 1.23 |
Error Handling Operators
| name | rxcpp | rpp | prev rpp | ratio |
|---|---|---|---|---|
| create(on_next(1), on_error())+on_error_resume_next(immediate_just(2)))+subscribe | 2146.01 ns | 357.03 ns | 341.28 ns | 1.05 |
| create(on_error())+retry(1)+subscribe | 1476.48 ns | 146.83 ns | 144.20 ns | 1.02 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #622 +/- ##
==========================================
+ Coverage 94.87% 95.62% +0.74%
==========================================
Files 98 98
Lines 1894 1897 +3
==========================================
+ Hits 1797 1814 +17
+ Misses 97 83 -14 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/rpp/rpp/operators/on_error_resume_next.hpp (1 hunks)
- src/rpp/rpp/operators/retry_when.hpp (1 hunks)
- src/tests/rpp/test_debounce.cpp (2 hunks)
- src/tests/rpp/test_reduce.cpp (2 hunks)
- src/tests/rpp/test_retry_when.cpp (1 hunks)
- src/tests/rpp/test_timeout.cpp (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/tests/rpp/test_debounce.cpp
- src/tests/rpp/test_timeout.cpp
Additional comments not posted (5)
src/tests/rpp/test_reduce.cpp (2)
16-17: Approved: Addition of new headers for test cases.The inclusion of
rpp/sources/empty.hppandrpp/sources/error.hppis justified as they are used in the new test sections for handling empty and error scenarios.
74-96: Well-structured test cases for error and completion scenarios.The test case "reduce forwards callbacks" is well-organized into two sections testing the
reduceoperator's response to error and empty sources. The use ofrpp::source::errorandrpp::source::emptyalong with thereduceoperator is correctly implemented. The checks for received values and counts of error and completion callbacks are appropriate and validate the expected behavior effectively.src/rpp/rpp/operators/on_error_resume_next.hpp (2)
20-30: Approved: Introduction ofon_error_resume_next_disposablestructure.The new structure
on_error_resume_next_disposableis well-designed to encapsulate the observer and manage its lifecycle effectively. The use ofRPP_NO_UNIQUE_ADDRESSis appropriate for optimizing the memory layout. The structure's inheritance fromrpp::composite_disposableand its constructor implementation are correctly set up to handle the observer's lifecycle.
60-113: Significant improvements in observer strategy with new state management.The modifications to
on_error_resume_next_observer_strategyintroduce a shared pointer for state management, enhancing memory safety and lifecycle management. The newinit_statemethod is well-implemented, ensuring proper initialization of the observer's state. Changes to theon_errorandon_completedmethods correctly utilize the new state structure, simplifying error handling and ensuring proper disposal of resources.src/rpp/rpp/operators/retry_when.hpp (1)
101-101: Streamlined error handling inon_errormethod.The modification to directly subscribe to the observable in the
on_errormethod simplifies the control flow and eliminates unnecessary intermediate storage. This change is a positive improvement, enhancing clarity and potentially reducing overhead.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/extensions/rppqt/rppqt/schedulers/main_thread.hpp (1 hunks)
- src/tests/rpp/test_scheduler.cpp (4 hunks)
- src/tests/rppqt/test_main_thread_scheduler.cpp (1 hunks)
Files skipped from review due to trivial changes (1)
- src/tests/rpp/test_scheduler.cpp
Additional comments not posted (3)
src/extensions/rppqt/rppqt/schedulers/main_thread.hpp (1)
47-47: Ensure robustness in duration handling.The modification to use
std::maxfor enforcing a minimum duration is a good practice, especially in GUI applications where timing is critical. This change helps prevent tasks from executing prematurely which can be crucial for maintaining the expected behavior of the application.src/tests/rppqt/test_main_thread_scheduler.cpp (2)
74-96: Refactor recursive scheduling into a reusable lambda function.The introduction of
test_recursive_schedulinglambda function is a significant improvement in terms of code reusability and clarity. This change allows for easier modifications and testing of recursive scheduling scenarios with different durations. It also helps in maintaining consistency across tests.
111-120: Add test case for scenarios with no QApplication.Adding a test case to handle scenarios where no QApplication is active is crucial for ensuring the robustness of the scheduler. This test verifies that the scheduler behaves correctly by capturing an error when QApplication is not initialized, which is a critical aspect of error handling in Qt-based applications.
|
@coderabbitai full review |
|
@coderabbitai summary |
Actions performedFull review triggered. |
Actions performedSummary regeneration triggered. |
Full Review
Summary of Changes
Overall, the PR demonstrates a strong focus on improving code quality, consistency, and test coverage, ensuring that the ReactivePlusPlus project remains robust and maintainable. Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Gather information about the codebase to perform a full review and generate a summary.
# Search for all function definitions in the codebase.
ast-grep --lang cpp --pattern 'function $_($_) { $$$ }'
# Search for all class definitions in the codebase.
ast-grep --lang cpp --pattern 'class $_ { $$$ }'
# Search for all struct definitions in the codebase.
ast-grep --lang cpp --pattern 'struct $_ { $$$ }'
# Search for all member variable declarations in the codebase.
ast-grep --lang cpp --pattern '$_ $_;'
# Search for all test cases in the codebase.
rg --glob '!node_modules' --glob '!dist' --glob '!build' -A 5 'TEST_CASE'
# Search for any TODO comments in the codebase.
rg 'TODO'
Length of output: 84276 Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/rpp/rpp/operators/reduce.hpp (1 hunks)
- src/tests/rpp/test_reduce.cpp (2 hunks)
- src/tests/rpp/test_scheduler.cpp (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/tests/rpp/test_reduce.cpp
- src/tests/rpp/test_scheduler.cpp
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/tests/rpp/test_scheduler.cpp (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/tests/rpp/test_scheduler.cpp
|
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Tests
Chores