signature: reorganize code

In SignatureInfo, Signature, and its subclasses:

* Improve Doxygen.
* Improve error messages.
* Make comparison operators non-member functions.

refs #4171

Change-Id: I9395a72594702255b41e3700ee145f35fc1a41f2
diff --git a/src/security/digest-sha256.cpp b/src/security/digest-sha256.cpp
index bbe2e10..3c8867c 100644
--- a/src/security/digest-sha256.cpp
+++ b/src/security/digest-sha256.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -32,7 +32,7 @@
   : Signature(signature)
 {
   if (getType() != tlv::DigestSha256)
-    BOOST_THROW_EXCEPTION(Error("Incorrect signature type"));
+    BOOST_THROW_EXCEPTION(Error("Cannot construct DigestSha256 from SignatureType " + to_string(getType())));
 }
 
 } // namespace ndn
diff --git a/src/security/digest-sha256.hpp b/src/security/digest-sha256.hpp
index d3a15de..fe226d8 100644
--- a/src/security/digest-sha256.hpp
+++ b/src/security/digest-sha256.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2014 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -22,34 +22,29 @@
 #ifndef NDN_SECURITY_DIGEST_SHA256_HPP
 #define NDN_SECURITY_DIGEST_SHA256_HPP
 
-#include "../data.hpp"
-#include "../encoding/tlv.hpp"
+#include "../signature.hpp"
 
 namespace ndn {
 
-/**
- * Represent a SHA256 digest.
+/** @brief Represents a signature of DigestSha256 type
+ *
+ *  This signature type provides integrity protection using SHA-256 digest, but no provenance of a
+ *  Data packet or any kind of guarantee that packet is from the original source.
  */
 class DigestSha256 : public Signature
 {
 public:
-  class Error : public Signature::Error
-  {
-  public:
-    explicit
-    Error(const std::string& what)
-      : Signature::Error(what)
-    {
-    }
-  };
-
+  /** @brief Create empty DigestSha256 signature
+   */
   DigestSha256();
 
+  /** @brief Convert base Signature to DigestSha256 signature
+   *  @throw Signature::Error SignatureType is not DigestSha256
+   */
   explicit
   DigestSha256(const Signature& signature);
-
 };
 
 } // namespace ndn
 
-#endif //NDN_SECURITY_DIGEST_SHA256_HPP
+#endif // NDN_SECURITY_DIGEST_SHA256_HPP
diff --git a/src/security/signature-sha256-with-ecdsa.cpp b/src/security/signature-sha256-with-ecdsa.cpp
index 72a5acd..bb86839 100644
--- a/src/security/signature-sha256-with-ecdsa.cpp
+++ b/src/security/signature-sha256-with-ecdsa.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -32,17 +32,17 @@
   : Signature(signature)
 {
   if (getType() != tlv::SignatureSha256WithEcdsa)
-    BOOST_THROW_EXCEPTION(Error("Incorrect signature type"));
+    BOOST_THROW_EXCEPTION(Error("Cannot construct Sha256WithEcdsa from SignatureType " + to_string(getType())));
 
   if (!hasKeyLocator()) {
-    BOOST_THROW_EXCEPTION(Error("KeyLocator is missing"));
+    BOOST_THROW_EXCEPTION(Error("KeyLocator is missing in Sha256WithEcdsa signature"));
   }
 }
 
 void
 SignatureSha256WithEcdsa::unsetKeyLocator()
 {
-  BOOST_THROW_EXCEPTION(Error("KeyLocator cannot be reset for SignatureSha256WithEcdsa"));
+  BOOST_THROW_EXCEPTION(Error("KeyLocator cannot be unset in Sha256WithEcdsa signature"));
 }
 
 } // namespace ndn
diff --git a/src/security/signature-sha256-with-ecdsa.hpp b/src/security/signature-sha256-with-ecdsa.hpp
index 4b0eb7f..c75ffc9 100644
--- a/src/security/signature-sha256-with-ecdsa.hpp
+++ b/src/security/signature-sha256-with-ecdsa.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2014 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -26,33 +26,32 @@
 
 namespace ndn {
 
-/**
- * represents a Sha256WithEcdsa signature.
+/** @brief Represents a signature of Sha256WithEcdsa type
+ *
+ *  This signature type provides integrity and provenance protection using an ECDSA signature over a
+ *  SHA-256 digest.
  */
 class SignatureSha256WithEcdsa : public Signature
 {
 public:
-  class Error : public Signature::Error
-  {
-  public:
-    explicit
-    Error(const std::string& what)
-      : Signature::Error(what)
-    {
-    }
-  };
-
+  /** @brief Create Sha256WithEcdsa signature with specified KeyLocator
+   */
   explicit
   SignatureSha256WithEcdsa(const KeyLocator& keyLocator = KeyLocator());
 
+  /** @brief Convert base Signature to Sha256WithEcdsa signature
+   *  @throw Signature::Error SignatureType is not Sha256WithEcdsa
+   */
   explicit
   SignatureSha256WithEcdsa(const Signature& signature);
 
 private:
+  /** @brief Prevent unsetting KeyLocator
+   */
   void
   unsetKeyLocator();
 };
 
 } // namespace ndn
 
-#endif //NDN_SECURITY_SIGNATURE_SHA256_WITH_ECDSA_HPP
+#endif // NDN_SECURITY_SIGNATURE_SHA256_WITH_ECDSA_HPP
diff --git a/src/security/signature-sha256-with-rsa.cpp b/src/security/signature-sha256-with-rsa.cpp
index 72f5c8b..97946f8 100644
--- a/src/security/signature-sha256-with-rsa.cpp
+++ b/src/security/signature-sha256-with-rsa.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -32,17 +32,17 @@
   : Signature(signature)
 {
   if (getType() != tlv::SignatureSha256WithRsa)
-    BOOST_THROW_EXCEPTION(Error("Incorrect signature type"));
+    BOOST_THROW_EXCEPTION(Error("Cannot construct Sha256WithRsa from SignatureType " + to_string(getType())));
 
   if (!hasKeyLocator()) {
-    BOOST_THROW_EXCEPTION(Error("KeyLocator is missing"));
+    BOOST_THROW_EXCEPTION(Error("KeyLocator is missing in Sha256WithRsa signature"));
   }
 }
 
 void
 SignatureSha256WithRsa::unsetKeyLocator()
 {
-  BOOST_THROW_EXCEPTION(Error("KeyLocator cannot be reset for SignatureSha256WithRsa"));
+  BOOST_THROW_EXCEPTION(Error("KeyLocator cannot be unset in Sha256WithRsa signature"));
 }
 
 } // namespace ndn
diff --git a/src/security/signature-sha256-with-rsa.hpp b/src/security/signature-sha256-with-rsa.hpp
index 937794b..2466018 100644
--- a/src/security/signature-sha256-with-rsa.hpp
+++ b/src/security/signature-sha256-with-rsa.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2014 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -26,33 +26,32 @@
 
 namespace ndn {
 
-/**
- * Represent a SHA256-with-RSA signature.
+/** @brief Represents a signature of Sha256WithRsa type
+ *
+ *  This signature type provides integrity and provenance protection using a RSA signature over a
+ *  SHA-256 digest.
  */
 class SignatureSha256WithRsa : public Signature
 {
 public:
-  class Error : public Signature::Error
-  {
-  public:
-    explicit
-    Error(const std::string& what)
-      : Signature::Error(what)
-    {
-    }
-  };
-
+  /** @brief Create Sha256WithRsa signature with specified KeyLocator
+   */
   explicit
   SignatureSha256WithRsa(const KeyLocator& keyLocator = KeyLocator());
 
+  /** @brief Convert base Signature to Sha256WithRsa signature
+   *  @throw Signature::Error SignatureType is not Sha256WithRsa
+   */
   explicit
   SignatureSha256WithRsa(const Signature& signature);
 
 private:
+  /** @brief Prevent unsetting KeyLocator
+   */
   void
   unsetKeyLocator();
 };
 
 } // namespace ndn
 
-#endif //NDN_SECURITY_SIGNATURE_SHA256_WITH_RSA_HPP
+#endif // NDN_SECURITY_SIGNATURE_SHA256_WITH_RSA_HPP
diff --git a/src/signature-info.cpp b/src/signature-info.cpp
index 3d789dc..1fc68f9 100644
--- a/src/signature-info.cpp
+++ b/src/signature-info.cpp
@@ -1,5 +1,5 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
  * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -23,8 +23,6 @@
 #include "encoding/block-helpers.hpp"
 #include "util/concepts.hpp"
 
-#include <boost/lexical_cast.hpp>
-
 namespace ndn {
 
 BOOST_CONCEPT_ASSERT((boost::EqualityComparable<SignatureInfo>));
@@ -58,93 +56,19 @@
   wireDecode(block);
 }
 
-void
-SignatureInfo::setSignatureType(tlv::SignatureTypeValue type)
-{
-  m_wire.reset();
-  m_type = type;
-}
-
-void
-SignatureInfo::setKeyLocator(const KeyLocator& keyLocator)
-{
-  m_wire.reset();
-  m_keyLocator = keyLocator;
-  m_hasKeyLocator = true;
-}
-
-void
-SignatureInfo::unsetKeyLocator()
-{
-  m_wire.reset();
-  m_keyLocator = KeyLocator();
-  m_hasKeyLocator = false;
-}
-
-const KeyLocator&
-SignatureInfo::getKeyLocator() const
-{
-  if (m_hasKeyLocator)
-    return m_keyLocator;
-  else
-    BOOST_THROW_EXCEPTION(Error("KeyLocator does not exist"));
-}
-
-void
-SignatureInfo::setValidityPeriod(const security::ValidityPeriod& validityPeriod)
-{
-  unsetValidityPeriod();
-  m_otherTlvs.push_front(validityPeriod.wireEncode());
-}
-
-void
-SignatureInfo::unsetValidityPeriod()
-{
-  m_wire.reset();
-  if (!m_otherTlvs.empty() && m_otherTlvs.front().type() == tlv::ValidityPeriod) {
-    m_otherTlvs.erase(m_otherTlvs.begin());
-  }
-}
-
-security::ValidityPeriod
-SignatureInfo::getValidityPeriod() const
-{
-  if (m_otherTlvs.empty() || m_otherTlvs.front().type() != tlv::ValidityPeriod) {
-    BOOST_THROW_EXCEPTION(Error("SignatureInfo does not contain the requested ValidityPeriod field"));
-  }
-
-  return security::ValidityPeriod(m_otherTlvs.front());
-}
-
-void
-SignatureInfo::appendTypeSpecificTlv(const Block& block)
-{
-  m_wire.reset();
-  m_otherTlvs.push_back(block);
-}
-
-
-const Block&
-SignatureInfo::getTypeSpecificTlv(uint32_t type) const
-{
-  for (std::list<Block>::const_iterator i = m_otherTlvs.begin();
-       i != m_otherTlvs.end(); i++) {
-    if (i->type() == type)
-      return *i;
-  }
-
-  BOOST_THROW_EXCEPTION(Error("(SignatureInfo::getTypeSpecificTlv) Requested a non-existed type [" +
-                              boost::lexical_cast<std::string>(type) + "] from SignatureInfo"));
-}
-
 template<encoding::Tag TAG>
 size_t
 SignatureInfo::wireEncode(EncodingImpl<TAG>& encoder) const
 {
+  // SignatureInfo ::= SIGNATURE-INFO-TLV TLV-LENGTH
+  //                     SignatureType
+  //                     KeyLocator?
+  //                     ValidityPeriod? (if present, stored as first item of m_otherTlvs)
+  //                     other SignatureType-specific sub-elements*
+
   size_t totalLength = 0;
 
-  for (std::list<Block>::const_reverse_iterator i = m_otherTlvs.rbegin();
-       i != m_otherTlvs.rend(); i++) {
+  for (auto i = m_otherTlvs.rbegin(); i != m_otherTlvs.rend(); i++) {
     totalLength += encoder.prependBlock(*i);
   }
 
@@ -164,7 +88,6 @@
 template size_t
 SignatureInfo::wireEncode<encoding::EstimatorTag>(EncodingImpl<encoding::EstimatorTag>& encoder) const;
 
-
 const Block&
 SignatureInfo::wireEncode() const
 {
@@ -184,50 +107,122 @@
 void
 SignatureInfo::wireDecode(const Block& wire)
 {
-  if (!wire.hasWire()) {
-    BOOST_THROW_EXCEPTION(Error("The supplied block does not contain wire format"));
-  }
-
   m_hasKeyLocator = false;
+  m_otherTlvs.clear();
 
   m_wire = wire;
   m_wire.parse();
 
   if (m_wire.type() != tlv::SignatureInfo)
-    BOOST_THROW_EXCEPTION(tlv::Error("Unexpected TLV type when decoding Name"));
+    BOOST_THROW_EXCEPTION(Error("Decoding SignatureInfo, but TLV-TYPE is " + to_string(m_wire.type())));
 
   Block::element_const_iterator it = m_wire.elements_begin();
 
   // the first block must be SignatureType
   if (it != m_wire.elements_end() && it->type() == tlv::SignatureType) {
     m_type = readNonNegativeInteger(*it);
-    it++;
+    ++it;
   }
   else
-    BOOST_THROW_EXCEPTION(Error("SignatureInfo does not have sub-TLV or the first sub-TLV is not "
-                                "SignatureType"));
+    BOOST_THROW_EXCEPTION(Error("Missing SignatureType in SignatureInfo"));
 
   // the second block could be KeyLocator
   if (it != m_wire.elements_end() && it->type() == tlv::KeyLocator) {
     m_keyLocator.wireDecode(*it);
     m_hasKeyLocator = true;
-    it++;
+    ++it;
   }
 
-  // Decode the rest of type-specific TLVs, if any
+  // store SignatureType-specific sub-elements, if any
   while (it != m_wire.elements_end()) {
     m_otherTlvs.push_back(*it);
-    it++;
+    ++it;
   }
 }
 
-bool
-SignatureInfo::operator==(const SignatureInfo& rhs) const
+void
+SignatureInfo::setSignatureType(tlv::SignatureTypeValue type)
 {
-  return (m_type == rhs.m_type &&
-          m_hasKeyLocator == rhs.m_hasKeyLocator &&
-          m_keyLocator == rhs.m_keyLocator &&
-          m_otherTlvs == rhs.m_otherTlvs);
+  m_wire.reset();
+  m_type = type;
+}
+
+const KeyLocator&
+SignatureInfo::getKeyLocator() const
+{
+  if (m_hasKeyLocator)
+    return m_keyLocator;
+  else
+    BOOST_THROW_EXCEPTION(Error("KeyLocator does not exist in SignatureInfo"));
+}
+
+void
+SignatureInfo::setKeyLocator(const KeyLocator& keyLocator)
+{
+  m_wire.reset();
+  m_keyLocator = keyLocator;
+  m_hasKeyLocator = true;
+}
+
+void
+SignatureInfo::unsetKeyLocator()
+{
+  m_wire.reset();
+  m_keyLocator = KeyLocator();
+  m_hasKeyLocator = false;
+}
+
+security::ValidityPeriod
+SignatureInfo::getValidityPeriod() const
+{
+  if (m_otherTlvs.empty() || m_otherTlvs.front().type() != tlv::ValidityPeriod) {
+    BOOST_THROW_EXCEPTION(Error("ValidityPeriod does not exist in SignatureInfo"));
+  }
+
+  return security::ValidityPeriod(m_otherTlvs.front());
+}
+
+void
+SignatureInfo::setValidityPeriod(const security::ValidityPeriod& validityPeriod)
+{
+  unsetValidityPeriod();
+  m_otherTlvs.push_front(validityPeriod.wireEncode());
+}
+
+void
+SignatureInfo::unsetValidityPeriod()
+{
+  if (!m_otherTlvs.empty() && m_otherTlvs.front().type() == tlv::ValidityPeriod) {
+    m_otherTlvs.pop_front();
+    m_wire.reset();
+  }
+}
+
+const Block&
+SignatureInfo::getTypeSpecificTlv(uint32_t type) const
+{
+  for (const Block& block : m_otherTlvs) {
+    if (block.type() == type)
+      return block;
+  }
+
+  BOOST_THROW_EXCEPTION(Error("TLV-TYPE " + to_string(type) + " sub-element does not exist in SignatureInfo"));
+}
+
+void
+SignatureInfo::appendTypeSpecificTlv(const Block& block)
+{
+  m_wire.reset();
+  m_otherTlvs.push_back(block);
+}
+
+bool
+operator==(const SignatureInfo& lhs, const SignatureInfo& rhs)
+{
+  return lhs.m_type == rhs.m_type &&
+         lhs.m_hasKeyLocator == rhs.m_hasKeyLocator &&
+         lhs.m_keyLocator == rhs.m_keyLocator &&
+         lhs.m_otherTlvs == rhs.m_otherTlvs;
 }
 
 std::ostream&
diff --git a/src/signature-info.hpp b/src/signature-info.hpp
index fc99542..7ac8404 100644
--- a/src/signature-info.hpp
+++ b/src/signature-info.hpp
@@ -1,5 +1,5 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
  * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -22,13 +22,14 @@
 #ifndef NDN_SIGNATURE_INFO_HPP
 #define NDN_SIGNATURE_INFO_HPP
 
-#include "encoding/tlv.hpp"
 #include "key-locator.hpp"
 #include "security/validity-period.hpp"
 #include <list>
 
 namespace ndn {
 
+/** @brief Represents a SignatureInfo TLV element
+ */
 class SignatureInfo
 {
 public:
@@ -42,101 +43,109 @@
     }
   };
 
+  /** @brief Create an invalid SignatureInfo
+   */
   SignatureInfo();
 
+  /** @brief Create with specified type
+   */
   explicit
   SignatureInfo(tlv::SignatureTypeValue type);
 
+  /** @brief Create with specified type and KeyLocator
+   */
   SignatureInfo(tlv::SignatureTypeValue type, const KeyLocator& keyLocator);
 
-  /**
-   * @brief Generate SignatureInfo from a block
-   *
-   * @throws tlv::Error if supplied block is not formatted correctly
+  /** @brief Create from wire encoding
+   *  @throw tlv::Error decode error
    */
   explicit
-  SignatureInfo(const Block& block);
+  SignatureInfo(const Block& wire);
 
-  /// @brief Set SignatureType
+  /** @brief Fast encoding or block size estimation
+   *  @param encoder EncodingEstimator or EncodingBuffer instance
+   */
+  template<encoding::Tag TAG>
+  size_t
+  wireEncode(EncodingImpl<TAG>& encoder) const;
+
+  /** @brief Encode to wire format
+   */
+  const Block&
+  wireEncode() const;
+
+  /** @brief Decode from wire format
+   *  @throw tlv::Error decode error
+   */
   void
-  setSignatureType(tlv::SignatureTypeValue type);
+  wireDecode(const Block& wire);
 
-  /// @brief Get SignatureType
+public: // field access
+  /** @brief Get SignatureType
+   *  @return tlv::SignatureTypeValue, or -1 to indicate invalid SignatureInfo
+   */
   int32_t
   getSignatureType() const
   {
     return m_type;
   }
 
-  /// @brief Check if KeyLocator is set
+  /** @brief Set SignatureType
+   */
+  void
+  setSignatureType(tlv::SignatureTypeValue type);
+
+  /** @brief Check if KeyLocator exists
+   */
   bool
   hasKeyLocator() const
   {
     return m_hasKeyLocator;
   }
 
-  /// @brief Set KeyLocator
-  void
-  setKeyLocator(const KeyLocator& keyLocator);
-
-  /// @brief Unset KeyLocator
-  void
-  unsetKeyLocator();
-
-  /**
-   * @brief Get KeyLocator
-   *
-   * @throws SignatureInfo::Error if keyLocator does not exist
+  /** @brief Get KeyLocator
+   *  @throw Error KeyLocator does not exist
    */
   const KeyLocator&
   getKeyLocator() const;
 
-  /// @brief Set ValidityPeriod
+  /** @brief Set KeyLocator
+   */
   void
-  setValidityPeriod(const security::ValidityPeriod& validityPeriod);
+  setKeyLocator(const KeyLocator& keyLocator);
 
-  /// @brief Unset ValidityPeriod
+  /** @brief Unset KeyLocator
+   */
   void
-  unsetValidityPeriod();
+  unsetKeyLocator();
 
-  /// @brief Get ValidityPeriod
+  /** @brief Get ValidityPeriod
+   *  @throw Error ValidityPeriod does not exist
+   */
   security::ValidityPeriod
   getValidityPeriod() const;
 
-  /// @brief Append signature type specific tlv block
+  /** @brief Set ValidityPeriod
+   */
   void
-  appendTypeSpecificTlv(const Block& block);
+  setValidityPeriod(const security::ValidityPeriod& validityPeriod);
 
-  /**
-   * @brief Get signature type specific tlv block
-   *
-   * @throws SignatureInfo::Error if the block does not exist
+  /** @brief Unset ValidityPeriod
+   */
+  void
+  unsetValidityPeriod();
+
+  /** @brief Get SignatureType-specific sub-element
+   *  @param type TLV-TYPE of sub-element
+   *  @throw Error sub-element of specified type does not exist
    */
   const Block&
   getTypeSpecificTlv(uint32_t type) const;
 
-  /// @brief Encode to a wire format or estimate wire format
-  template<encoding::Tag TAG>
-  size_t
-  wireEncode(EncodingImpl<TAG>& encoder) const;
-
-  /// @brief Encode to a wire format
-  const Block&
-  wireEncode() const;
-
-  /// @brief Decode from a wire format
+  /** @brief Append SignatureType-specific sub-element
+   */
   void
-  wireDecode(const Block& wire);
-
-public: // EqualityComparable concept
-  bool
-  operator==(const SignatureInfo& rhs) const;
-
-  bool
-  operator!=(const SignatureInfo& rhs) const
-  {
-    return !(*this == rhs);
-  }
+  appendTypeSpecificTlv(const Block& element);
 
 private:
   int32_t m_type;
@@ -146,10 +155,22 @@
 
   mutable Block m_wire;
 
+  friend bool
+  operator==(const SignatureInfo& lhs, const SignatureInfo& rhs);
+
   friend std::ostream&
   operator<<(std::ostream& os, const SignatureInfo& info);
 };
 
+bool
+operator==(const SignatureInfo& lhs, const SignatureInfo& rhs);
+
+inline bool
+operator!=(const SignatureInfo& lhs, const SignatureInfo& rhs)
+{
+  return !(lhs == rhs);
+}
+
 std::ostream&
 operator<<(std::ostream& os, const SignatureInfo& info);
 
diff --git a/src/signature.cpp b/src/signature.cpp
index af18ec2..3683449 100644
--- a/src/signature.cpp
+++ b/src/signature.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2015 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -50,7 +50,7 @@
 Signature::setValue(const Block& value)
 {
   if (value.type() != tlv::SignatureValue) {
-    BOOST_THROW_EXCEPTION(Error("The supplied block is not SignatureValue"));
+    BOOST_THROW_EXCEPTION(Error("Expecting SignatureValue, but TLV-TYPE is " + to_string(value.type())));
   }
   m_value = value;
 }
diff --git a/src/signature.hpp b/src/signature.hpp
index 4644807..315d061 100644
--- a/src/signature.hpp
+++ b/src/signature.hpp
@@ -1,5 +1,5 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
  * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -22,13 +22,17 @@
 #ifndef NDN_SIGNATURE_HPP
 #define NDN_SIGNATURE_HPP
 
-#include "common.hpp"
 #include "signature-info.hpp"
 
 namespace ndn {
 
-/**
- * A Signature is storage for the signature-related information (info and value) in a Data packet.
+/** @brief Holds SignatureInfo and SignatureValue in a Data packet
+ *
+ *  A Signature is not a TLV element itself. It collects SignatureInfo and SignatureValue TLV
+ *  elements together for easy access.
+ *  In most cases, an application should use a subclass of Signature such as @p DigestSha256 , @p
+ *  SignatureSha256WithRsa , or @p SignatureSha256WithEcdsa instead of using @p Signature type
+ *  directly.
  */
 class Signature
 {
@@ -51,22 +55,15 @@
   explicit
   Signature(const SignatureInfo& info, const Block& value = Block());
 
+  /** @brief Determine whether SignatureInfo is valid
+   */
+  explicit
   operator bool() const
   {
     return m_info.getSignatureType() != -1;
   }
 
-  /**
-   * @brief Get SignatureInfo in the wire format
-   */
-  const Block&
-  getInfo() const
-  {
-    return m_info.wireEncode(); // will do nothing if wire already exists
-  }
-
-  /**
-   * @brief Get SignatureInfo
+  /** @brief Get SignatureInfo
    */
   const SignatureInfo&
   getSignatureInfo() const
@@ -74,16 +71,21 @@
     return m_info;
   }
 
-  /**
-   * @brief Set SignatureInfo from a block
-   *
-   * @throws tlv::Error if supplied block is not formatted correctly
+  /** @brief Get SignatureInfo as wire format
+   */
+  const Block&
+  getInfo() const
+  {
+    return m_info.wireEncode();
+  }
+
+  /** @brief Decode SignatureInfo from wire format
+   *  @throw tlv::Error decode error
    */
   void
   setInfo(const Block& info);
 
-  /**
-   * @brief Set SignatureInfo
+  /** @brief Set SignatureInfo
    */
   void
   setInfo(const SignatureInfo& info)
@@ -91,26 +93,22 @@
     m_info = info;
   }
 
-  /**
-   * @brief Get SignatureValue in the wire format
+  /** @brief Get SignatureValue
    */
   const Block&
   getValue() const
   {
-    m_value.encode(); // will do nothing if wire already exists
     return m_value;
   }
 
-  /**
-   * @brief Get SignatureValue from a block
-   *
-   * @throws tlv::Error if supplied block has type different from SignatureValue
+  /** @brief Set SignatureValue
+   *  @throws tlv::Error TLV-TYPE of supplied block is not SignatureValue, or the block does not have TLV-VALUE
    */
   void
   setValue(const Block& value);
 
-  /**
-   * @brief Get signature type
+public: // SignatureInfo fields
+  /** @brief Get SignatureType
    */
   uint32_t
   getType() const
@@ -118,8 +116,7 @@
     return m_info.getSignatureType();
   }
 
-  /**
-   * @brief Check if SignatureInfo block has a KeyLocator
+  /** @brief Check if KeyLocator exists in SignatureInfo
    */
   bool
   hasKeyLocator() const
@@ -127,10 +124,8 @@
     return m_info.hasKeyLocator();
   }
 
-  /**
-   * @brief Get KeyLocator
-   *
-   * @throws Signature::Error if KeyLocator does not exist
+  /** @brief Get KeyLocator
+   *  @throw tlv::Error KeyLocator does not exist in SignatureInfo
    */
   const KeyLocator&
   getKeyLocator() const
@@ -138,8 +133,7 @@
     return m_info.getKeyLocator();
   }
 
-  /**
-   * @brief Set KeyLocator
+  /** @brief Set KeyLocator
    */
   void
   setKeyLocator(const KeyLocator& keyLocator)
@@ -147,11 +141,10 @@
     m_info.setKeyLocator(keyLocator);
   }
 
-  /**
-   * @brief Unset KeyLocator
+  /** @brief Unset KeyLocator
    *
-   * Note that specific signature types may provide advisory (non-virtual) override
-   * to prevent unsetting KeyLocator if it is required by the specification.
+   *  @note Subclasses of Signature may provide advisory (non-virtual) override to prevent unsetting
+   *        KeyLocator if it is required by the specification.
    */
   void
   unsetKeyLocator()
@@ -159,25 +152,23 @@
     m_info.unsetKeyLocator();
   }
 
-public: // EqualityComparable concept
-  bool
-  operator==(const Signature& other) const
-  {
-    return getInfo() == other.getInfo() &&
-      getValue() == other.getValue();
-  }
-
-  bool
-  operator!=(const Signature& other) const
-  {
-    return !(*this == other);
-  }
-
 protected:
   SignatureInfo m_info;
   mutable Block m_value;
 };
 
+inline bool
+operator==(const Signature& lhs, const Signature& rhs)
+{
+  return lhs.getInfo() == rhs.getInfo() && lhs.getValue() == rhs.getValue();
+}
+
+inline bool
+operator!=(const Signature& lhs, const Signature& rhs)
+{
+  return !(lhs == rhs);
+}
+
 } // namespace ndn
 
 #endif // NDN_SIGNATURE_HPP