[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #716: MINIFICPP-1127 - Provenance repo performance should be improved

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

szaszm commented on a change in pull request #716: MINIFICPP-1127 - Provenance repo performance should be improved
URL: https://github.com/apache/nifi-minifi-cpp/pull/716#discussion_r374657478
 
 

 ##########
 File path: libminifi/include/core/Repository.h
 ##########
 @@ -85,6 +85,10 @@ class Repository : public virtual core::SerializableComponent, public core::Trac
 
   virtual void flush();
 
+  virtual void printStats() {
+    return;
+  }
+
 
 Review comment:
   Introducing an empty function to a base class is a smell. In this case, `printStats` doesn't need to be `virtual` or even present in `Repository`.
   
   From the standpoint of contracts:
   By introducing `printStats` to `Repository`, `Repository` now looks like something that can print stats of itself. The reality is that only `ProvenanceRepository` (and potentially later a select other few derived classes) can offer this functionality. In all other cases, this is a broken promise.
   
   From the OOP standpoint:
   `printStats` is a method specific to `ProvenanceRepository` but has leaked to `Repository`, breaking encapsulation.
   
   From a performance standpoint:
   Why pay for virtual dispatch when in all of the callers of `printStats`, the static and the dynamic types of the object match? We could even go as far as removing the function declaration from the headers altogether and giving it `static` linkage, therefore giving the compiler the freedom to throw the implementation away (except for 1 inlined instance) to reduce code bloat.

----------------------------------------------------------------
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


With regards,
Apache Git Services