adamdebreceni commented on a change in pull request #839:
URL: https://github.com/apache/nifi-minifi-cpp/pull/839#discussion_r492072291
##########
File path: libminifi/include/utils/Id.h
##########
@@ -43,107 +44,41 @@ class uuid;
#define UUID_TIME_STR "time"
#define UUID_WINDOWS_STR "windows"
-
namespace org {
namespace apache {
namespace nifi {
namespace minifi {
namespace utils {
-template<typename T, typename C>
-class IdentifierBase {
+class Identifier {
+ friend struct IdentifierTestAccessor;
public:
- IdentifierBase(T myid) { // NOLINT
- copyInto(myid);
- }
-
- IdentifierBase(const IdentifierBase &other) {
- copyInto(other.id_);
- }
-
- IdentifierBase(IdentifierBase &&other)
- : converted_(std::move(other.converted_)) {
- copyInto(other.id_);
- }
-
- IdentifierBase() = default;
-
- IdentifierBase &operator=(const IdentifierBase &other) {
- copyInto(other.id_);
- return *this;
- }
-
- IdentifierBase &operator=(T o) {
- copyInto(o);
- return *this;
- }
+ using Data = std::array<uint8_t, 16>;
- void getIdentifier(T other) const {
- copyOutOf(other);
- }
+ Identifier() = default;
+ explicit Identifier(const Data& data);
+ explicit Identifier(const std::string& data);
+ Identifier &operator=(const Data& data);
- C convert() const {
- return converted_;
- }
+ Identifier &operator=(const std::string& idStr);
- protected:
- void copyInto(const IdentifierBase &other) {
- memcpy(id_, other.id_, sizeof(T));
- }
+ bool operator!=(const Identifier& other) const;
+ bool operator==(const Identifier& other) const;
- void copyInto(const void *other) {
- memcpy(id_, other, sizeof(T));
- }
-
- void copyOutOf(void *other) {
- memcpy(other, id_, sizeof(T));
- }
-
- C converted_{};
-
- T id_{};
-};
-
-typedef uint8_t UUID_FIELD[16];
-
-class Identifier : public IdentifierBase<UUID_FIELD, std::string> {
- public:
- Identifier(UUID_FIELD u); // NOLINT
- Identifier();
- Identifier(const Identifier &other);
- Identifier(Identifier &&other);
-
- /**
- * I believe these exist to make windows builds happy -- need more testing
- * to ensure this doesn't cause any issues.
- */
- Identifier(const IdentifierBase &other); // NOLINT
- Identifier &operator=(const IdentifierBase &other);
-
- Identifier &operator=(const Identifier &other);
- Identifier &operator=(UUID_FIELD o);
-
- Identifier &operator=(std::string id);
- bool operator==(const std::nullptr_t nullp) const;
-
- bool operator!=(std::nullptr_t nullp) const;
-
- bool operator!=(const Identifier &other) const;
- bool operator==(const Identifier &other) const;
+ bool isNil() const;
Review comment:
as far as I know the "nil" uuid is not "invalid", it is described as
only "special"
> The nil UUID is special form of UUID that is specified to have all
128 bits set to zero.
the fact that we treat it as invalid is unfortunate and in the future we
should use `optional<Identifier>` wherever possible, in fact I've already tried
to propagate `optional<Identifier>` but the diff blew up and since it is a
semi-unrelated change I've decided to drop it and take it up in an other ticket
----------------------------------------------------------------
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]
