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

Consolidate reactor timer management functions #2301

Merged
merged 3 commits into from
Nov 22, 2024

Conversation

sonndinh
Copy link
Contributor

@sonndinh sonndinh commented Nov 19, 2024

  • Problem: ACE_Reactor and its base class, ACE_Reactor_Timer_Interface have identical implementations for schedule_timer and reset_timer_interval member template functions.

  • Solution: Bring the base class's member functions into the scope of the ACE_Reactor class with using.

The original attempt was to remove the duplicate member functions from ACE_Reactor while keeping the ones from the base class. However, this does not work since the base class's member template functions are not visible in ACE_Reactor with the presence of the overloads for schedule_timer and reset_timer_interval with ACE_Time_Value. This change brings the base class's schedule_timer and reset_timer_interval member templates into the scope of the ACE_Reactor class so that we no longer need the identical implementations in ACE_Reactor.

@sonndinh sonndinh marked this pull request as draft November 19, 2024 22:32
@sonndinh sonndinh marked this pull request as ready for review November 21, 2024 18:08
@sonndinh sonndinh changed the title Remove duplicate reactor timer management functions Consolidate reactor timer management functions Nov 21, 2024
@sonndinh
Copy link
Contributor Author

@mitza-oci Using using is better - it's more concise and saves a call to the base class's member function.

@jrw972 jrw972 merged commit 117ca73 into DOCGroup:master Nov 22, 2024
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants