[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #824: MINIFICPP-1256 - Apply fixes recommended by modernize-equals-default clang tidy check

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

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



##########
File path: extensions/jni/jvm/JavaClass.h
##########
@@ -46,9 +46,7 @@ class JavaClass {
  public:
 
   JavaClass()
-      : class_ref_(nullptr) {
-
-  }
+      : class_ref_(nullptr) = default;

Review comment:
       This looks incorrect to me.

##########
File path: libminifi/include/core/state/UpdatePolicy.h
##########
@@ -69,9 +69,7 @@ class UpdatePolicy {
   }
 
  protected:
-  explicit UpdatePolicy(const UpdatePolicy &other)
-      : enable_all_(other.enable_all_), properties_(other.properties_) {
-  }
+  explicit UpdatePolicy(const UpdatePolicy &other) = default;

Review comment:
       Would you mind fixing the copy and move ctors by removing the `explicit` specifiers and making the move ctor and assignment operators take mutable rvalue reference? Not strictly related to your changes, but a trivial fix nearby.

##########
File path: extensions/jni/jvm/JniProcessContext.h
##########
@@ -39,8 +39,7 @@ struct JniProcessContext {
       : nifi_processor_(nullptr),
         cslookup_(nullptr),
         processor_(nullptr),
-        context_(nullptr) {
-  }
+        context_(nullptr) = default;

Review comment:
       This is incorrect

##########
File path: extensions/openwsman/processors/SourceInitiatedSubscriptionListener.cpp
##########
@@ -159,8 +159,7 @@ SourceInitiatedSubscriptionListener::Handler::Handler(SourceInitiatedSubscriptio
 
 SourceInitiatedSubscriptionListener::SubscriberData::SubscriberData()
     : bookmark_(nullptr)
-    , subscription_(nullptr) {
-}
+    , subscription_(nullptr) = default;

Review comment:
       This is an error

##########
File path: extensions/windows-event-log/wel/WindowsEventLog.h
##########
@@ -63,8 +63,7 @@ typedef std::vector<std::pair<METADATA, std::string>> METADATA_NAMES;
 class WindowsEventLogHandler
 {
 public:
-  WindowsEventLogHandler() : metadata_provider_(nullptr){
-  }
+  WindowsEventLogHandler() : metadata_provider_(nullptr) = default;

Review comment:
       This is an error

##########
File path: libminifi/include/core/state/UpdatePolicy.h
##########
@@ -122,10 +120,7 @@ class UpdatePolicyBuilder {
 
 
  protected:
-  explicit UpdatePolicyBuilder(const UpdatePolicyBuilder &other)
-      : current_policy_(other.current_policy_) {
-  }
-
+  explicit UpdatePolicyBuilder(const UpdatePolicyBuilder &other) = default;

Review comment:
       Would you mind fixing this copy ctor by removing the `explicit` specifier?

##########
File path: extensions/opencv/FrameIO.h
##########
@@ -30,7 +30,7 @@ class FrameWriteCallback : public OutputStreamCallback {
     // TODO - Nghia: Check std::move(img_mat).
         : image_mat_(std::move(image_mat)), image_encoding_(image_encoding_) {
     }
-    ~FrameWriteCallback() override = default;
+    ~FrameWriteCallback() = default;

Review comment:
       Please restore the `override` specifier

##########
File path: libminifi/include/core/Relationship.h
##########
@@ -78,11 +74,7 @@ class Relationship {
     return name_ < right.name_;
   }
 
-  Relationship &operator=(const Relationship &other) {
-    name_ = other.name_;
-    description_ = other.description_;
-    return *this;
-  }
+  Relationship &operator=(const Relationship &other) = default;

Review comment:
       :+1:
   
   Not only did you simplify the code, but you also handled self-assignment of many classes, including this one. Thanks.

##########
File path: libminifi/include/core/state/FlowIdentifier.h
##########
@@ -68,12 +68,7 @@ class FlowIdentifier {
     bucket_id_ = other.bucket_id_;
     flow_id_ = other.flow_id_;
   }
-  FlowIdentifier &operator=(const FlowIdentifier &other) {
-    registry_url_ = other.registry_url_;
-    bucket_id_ = other.bucket_id_;
-    flow_id_ = other.flow_id_;
-    return *this;
-  }
+  FlowIdentifier &operator=(const FlowIdentifier &other) = default;

Review comment:
       Please `=default` the "copy ctor" above as well and remove its `explicit` specifier.




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