nifi-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #716: MINIFICPP-1127 - Provenance repo performance should be improved
Date Tue, 04 Feb 2020 13:12:50 GMT
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

Mime
View raw message