[
https://issues.apache.org/jira/browse/MINIFICPP-676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16688658#comment-16688658
]
ASF GitHub Bot commented on MINIFICPP-676:
------------------------------------------
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/440#discussion_r234008835
--- Diff: libminifi/include/io/Serializable.h ---
@@ -173,8 +147,22 @@ class Serializable {
**/
int readUTF(std::string &str, DataStream *stream, bool widen = false);
- protected:
+ /**
+ * reads 2-8 bytes from the stream
+ * @param value reference in which will set the result
+ * @param stream stream from which we will read
+ * @return resulting read size
+ **/
+ template<typename Integral, typename std::enable_if<
+ (sizeof(Integral) > 1) &&
+ std::is_integral<Integral>::value &&
+ !std::is_signed<Integral>::value
+ ,Integral>::type* = nullptr>
+ int read(Integral &value, DataStream *stream, bool is_little_endian =
EndiannessCheck::IS_LITTLE) {
--- End diff --
My IDEs does not make this clear the function to use. In some ways java
made this easier with their streams "writeInt, writeShort, writeUnsignedShort"
and we can do the same with write(short) write(int), write(long), but my one
IDE correctly provides this while others don't. I agree that generally
reducing "Needless functions" is a good thing, but here it served a purpose.
Happy to further discuss how we can benefit that purpose.
> Cleanup and fix serializable interface implementation
> -----------------------------------------------------
>
> Key: MINIFICPP-676
> URL: https://issues.apache.org/jira/browse/MINIFICPP-676
> Project: NiFi MiNiFi C++
> Issue Type: Improvement
> Reporter: Arpad Boda
> Assignee: Arpad Boda
> Priority: Minor
>
> Serializable interface contains a couple of issues:
> Type-unsafe template functions
> Code duplication
> Needless functions
> The goal of this ticket is to make the interface cleaner and simpler.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
