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



##########
File path: extensions/sftp/client/SFTPClient.h
##########
@@ -49,6 +50,12 @@ enum class SFTPError : uint8_t {
   SFTP_ERROR_UNEXPECTED
 };
 
+struct SFTPException : Exception {
+  explicit SFTPException(const SFTPError err)
+      :Exception{ExceptionType::FILE_OPERATION_EXCEPTION, fmt::format("SFTP 
Error {0}", static_cast<int>(err))}

Review comment:
       I have some negative feedback regarding the conversion to SMART_ENUM: 
the enum values are implicitly convertible to integer which makes it less safe 
than `enum class`. This resulted in compiling but invalid code when the wrong 
overload of `LastSFTPError::operator=` was called and I had to introduce 
differently named functions.
   
   I'm leaning towards keeping SMART_ENUM in this case but I'll be more careful 
about introducing it in new places, weighing the convenient string conversion 
against the safety tradeoff in each case.




----------------------------------------------------------------
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:
[email protected]


Reply via email to