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 #713: MINIFICPP-1119 unify win/posix sockets + clean up issues
Date Mon, 10 Feb 2020 16:45:02 GMT
szaszm commented on a change in pull request #713: MINIFICPP-1119 unify win/posix sockets +
clean up issues
URL: https://github.com/apache/nifi-minifi-cpp/pull/713#discussion_r377182058
 
 

 ##########
 File path: libminifi/include/io/NetworkPrioritizer.h
 ##########
 @@ -83,29 +66,26 @@ class NetworkInterface {
     }
   }
 
-  NetworkInterface &operator=(const NetworkInterface &&other) {
-    ifc_ = std::move(other.ifc_);
-    prioritizer_ = std::move(other.prioritizer_);
-    return *this;
-  }
+  NetworkInterface &operator=(const NetworkInterface &other) = default;
+  NetworkInterface &operator=(NetworkInterface &&other) noexcept(std::is_nothrow_move_assignable<std::string>::value)
= default;
 
 Review comment:
   I'm OK with either keeping or removing `noexcept`, so I'll remove it. My arguments
   * For keeping: Even though string is not always marked noexcept move assignable, it will
never throw in practice and a shared_ptr move assignment doesn't throw either
   * Against keeping: string is not marked as noexcept move assignable before C++17 so even
though it will probably never throw in practice, we have no hard guarantee.
   
   In general about `noexcept`:
   > This is very nice until someone comes along and adds another class member whose move
assignment operator throws, but forgets to update this expression
   
   While your reasoning is correct, I believe this approach is too conservative. We already
require people to write reasonable code and depend on certain rules being followed without
compiler enforcement. Some examples that come to my mind: destructors must not throw, classes
meant for subclassing must have a virtual destructor, every resource needs to be freed, don't
return a reference to a local or temporary.
   
   One could make the argument that our program is only correct "until someone comes along
and" does something that violates one of these rules. Yet, we take some risks and do not ban
returning references in order to avoid accidentally returning dangling references.
   
   Often finding the right balance of benefits vs. risks is hard, especially when it comes
to newer features, so my strategy is to follow the style and best practices of the C++ community.
Generally people are more conservative with new things like `noexcept`, which can sometimes
be right, but it's not always the right approach IMO.

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