szaszm commented on a change in pull request #743: Minificpp 1169 - Simplify C2
metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r398035081
##########
File path: nanofi/include/cxx/C2CallbackAgent.h
##########
@@ -70,6 +69,7 @@ class C2CallbackAgent : public c2::C2Agent {
c2_ag_stop_callback *stop;
private:
+ utils::ThreadPool<utils::TaskRescheduleInfo> thread_pool_;
Review comment:
Since this class inherits from C2Agent, it should also inherit its
`ThreadPool`, so there should be no need for 2 `ThreadPool`s in
`C2CallbackAgent`.
Do you think it would make sense to revert C2Agent to take the thread pool
as a constructor argument instead of creating it, in the spirit of the "[Tell,
Don't Ask](https://www.martinfowler.com/bliki/TellDontAsk.html)" principle, the
"[Single-responsibility
principle](https://en.wikipedia.org/wiki/Single-responsibility_principle)"
(object creation is a reponsibility) and "[Inversion of
control](https://en.wikipedia.org/wiki/Inversion_of_control)"? This approach
would solve the issue by making it possible to reuse the same thread pool for
both classes, while leaving option to use separate thread pools if needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
