[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #974: MINIFICPP-1449 Add pause and resume command to C2

[GitBox <gi...@apache.org>]

szaszm commented on a change in pull request #974:
URL: https://github.com/apache/nifi-minifi-cpp/pull/974#discussion_r563694487



##########
File path: libminifi/include/core/state/UpdateController.h
##########
@@ -148,7 +148,16 @@ class UpdateRunner : public utils::AfterExecute<Update> {
   std::chrono::milliseconds delay_;
 };
 
-class StateController {
+class PausableStateController {
+ public:
+  virtual ~PausableStateController() = default;
+
+  virtual int16_t pause() = 0;
+
+  virtual int16_t resume() = 0;
+};

Review comment:
       There is nothing in this interface that is related to `StateController`. This interface should probably be named `Pausable` instead. This also implies some C2Agent and related changes. The C2 part probably doesn't need to care what kind of object it pauses, but just pause and resume something that is `Pausable` and was passed as the ctor argument to be paused/resumed.
   
   I was thinking about defining `Pausable` as a runtime concept with reference semantics, but that is only if you like the idea of not having to depend on/derive from a class to "be" `Pausable` enough to take on significantly increased implementation complexity. The implementation would be a wrapper class with a pointer to a private abstract class (concept) similar to the current one and a constructor template that instantiates a private class template (model) implementing the private concept class.




----------------------------------------------------------------
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:
users@infra.apache.org