From bab80d20c73cb260feaebe7a4be5f497a7f8450e Mon Sep 17 00:00:00 2001 From: Joshua Scott Date: Fri, 21 Dec 2018 01:20:52 +0000 Subject: [PATCH] pclass: Cleanup VectorProperty and fix build errors --- include/ki/pclass/Property.h | 8 +- include/ki/pclass/StaticProperty.h | 58 ++- include/ki/pclass/VectorProperty.h | 623 +++++++++++------------------ src/pclass/Property.cpp | 14 + 4 files changed, 271 insertions(+), 432 deletions(-) diff --git a/include/ki/pclass/Property.h b/include/ki/pclass/Property.h index fc117cd..2c89e7d 100644 --- a/include/ki/pclass/Property.h +++ b/include/ki/pclass/Property.h @@ -1,5 +1,9 @@ #pragma once +#include #include "ki/pclass/types/Type.h" +#include "ki/pclass/HashCalculator.h" +#include "ki/pclass/Value.h" +#include "ki/util/BitStream.h" namespace ki { @@ -42,8 +46,8 @@ namespace pclass virtual const PropertyClass *get_object(std::size_t index = 0) const = 0; virtual void set_object(std::unique_ptr &object, std::size_t index = 0) = 0; - virtual void write_value_to(BitStream &stream, std::size_t index = 0) const = 0; - virtual void read_value_from(BitStream &stream, std::size_t index = 0) = 0; + virtual void write_value_to(BitStream &stream, std::size_t index = 0) const; + virtual void read_value_from(BitStream &stream, std::size_t index = 0); private: const PropertyClass *m_instance; diff --git a/include/ki/pclass/StaticProperty.h b/include/ki/pclass/StaticProperty.h index 08b7c2e..06684c6 100644 --- a/include/ki/pclass/StaticProperty.h +++ b/include/ki/pclass/StaticProperty.h @@ -78,8 +78,10 @@ namespace pclass }; /** - * Provides default implementations of construct() and copy() - * for static_object_helper where ValueT is a pointer type. + * Provides default implementations of construct(), copy() + * get_object(), and set_object() for static_object_helper where + * ValueT is a pointer type. + * @tparam ValueT The type of the value (as a non-pointer type). */ template struct pointer_static_object_helper @@ -340,7 +342,8 @@ namespace pclass static void set_value(StaticProperty &prop, Value value, int index) { - prop.m_value = value.as().get(); + Value casted_value = value.as(); + prop.m_value = casted_value.get(); } }; @@ -356,12 +359,15 @@ namespace pclass { static Value get_value(const StaticProperty &prop, int index) { + if (prop.m_value == nullptr) + throw runtime_error("Called get_value() but value is nullptr."); return Value::make_reference(*prop.m_value); } static void set_value(StaticProperty &prop, Value value, int index) { - prop.m_value = value.as().release(); + Value casted_value = value.as(); + prop.m_value = casted_value.release(); } }; @@ -379,7 +385,8 @@ namespace pclass static void set_value(StaticProperty &prop, Value value, const int index) { - prop.m_value[index] = value.as().get(); + Value casted_value = value.as(); + prop.m_value[index] = casted_value.get(); } }; @@ -397,31 +404,26 @@ namespace pclass static void set_value(StaticProperty &prop, Value value, const int index) { - prop.m_value[index] = value.as().release(); + Value casted_value = value.as(); + prop.m_value[index] = casted_value.release(); } }; } /** - * TODO: Documentation + * Base type for StaticProperty. + * This is used to remove the amount of repeated code in specializations. */ template class IStaticProperty : public IProperty { public: + using IProperty::IProperty; + // Do not allow copy assignment. Once a property has been constructed, // it shouldn't be able to change. virtual IStaticProperty &operator=(const IStaticProperty &that) = delete; - IStaticProperty(PropertyClass &object, - const std::string &name, const Type &type) - : IProperty(object, name, type) - {} - - IStaticProperty(PropertyClass &object, const StaticProperty &that) - : IProperty(object, that) - {} - constexpr bool is_pointer() const override { return std::is_pointer::value; @@ -436,20 +438,6 @@ namespace pclass { throw runtime_error("Tried to call set_element_count() on a property that is not dynamic."); } - - void write_value_to(BitStream &stream, const std::size_t index) const override - { - if (index < 0 || index >= get_element_count()) - throw runtime_error("Index out of bounds."); - get_type().write_to(stream, get_value(index)); - } - - void read_value_from(BitStream &stream, const std::size_t index) override - { - if (index < 0 || index >= get_element_count()) - throw runtime_error("Index out of bounds."); - set_value(get_type().read_from(stream), index); - } }; /** @@ -471,19 +459,19 @@ namespace pclass StaticProperty(PropertyClass &object, const std::string &name, const Type &type) - : IStaticProperty(object, name, type) + : IStaticProperty(object, name, type) , m_value(detail::static_object_helper::construct(type)) {} StaticProperty(PropertyClass &object, const std::string &name, const Type &type, ValueT value) - : IStaticProperty(object, name, type) + : IStaticProperty(object, name, type) { m_value = value; } StaticProperty(PropertyClass &object, const StaticProperty &that) - : IStaticProperty(object, that) + : IStaticProperty(object, that) , m_value(detail::static_object_helper::copy(that)) {} @@ -576,14 +564,14 @@ namespace pclass StaticProperty(PropertyClass &object, const std::string &name, const Type &type) - : IStaticProperty(object, name, type) + : IStaticProperty(object, name, type) { for (auto i = 0; i < N; ++i) m_value[i] = detail::static_object_helper::construct(type); } StaticProperty(PropertyClass &object, const StaticProperty &that) - : IStaticProperty(object, that) + : IStaticProperty(object, that) { for (auto i = 0; i < N; ++i) m_value[i] = that.m_value[i]; diff --git a/include/ki/pclass/VectorProperty.h b/include/ki/pclass/VectorProperty.h index 298a359..dd222ce 100644 --- a/include/ki/pclass/VectorProperty.h +++ b/include/ki/pclass/VectorProperty.h @@ -1,5 +1,6 @@ #pragma once #include +#include #include "ki/pclass/Property.h" #include "ki/util/exception.h" @@ -11,397 +12,235 @@ namespace pclass template class VectorProperty; - /// @cond DOXYGEN_SKIP - /** - * A helper utility that provides the right implementation of copy(), - * get_object() and set_object(), based on characteristics of type: ValueT. - */ - template < - typename ValueT, - typename IsPointerEnable = void, - typename IsBaseEnable = void - > - struct vector_value_object_helper + namespace detail { - static ValueT copy(VectorProperty &prop, const int index) + /** + * Provides default implementations for vector_object_helper. + */ + template + struct default_vector_object_helper { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); + static ValueT copy(VectorProperty &prop, const int index) + { + // In cases where ValueT is not a pointer, and does not derive from PropertyClass, + // just call the copy constructor. + return ValueT(prop.at(index)); + } - // In cases where ValueT is not a pointer, and does not derive from PropertyClass, - // just call the copy constructor. - return ValueT(prop.at(index)); - } + static const PropertyClass *get_object(const VectorProperty &prop, const int index) + { + // ValueT does not derive from PropertyClass, and so, this property is not + // storing an object. + throw runtime_error( + "Tried calling get_object() on a property that does not store an object." + ); + } - static Value get_value(const VectorProperty &prop, const int index) + static void set_object(VectorProperty &prop, + std::unique_ptr &object, const int index) + { + // ValueT does not derive from PropertyClass, and so, this property is not + // storing an object. + throw runtime_error( + "Tried calling set_object() on a property that does not store an object." + ); + } + }; + + /** + * Provides default implementations of copy() + * for static_object_helper where ValueT is a pointer type. + * @tparam ValueT The type of the value (as a non-pointer type). + */ + template + struct pointer_vector_object_helper { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - return Value::make_reference(prop.at(index)); - } + static ValueT *copy(const VectorProperty &prop) + { + // The copy constructor for all pointers is to copy the pointer + // without creating a new copy of the object it's pointing to. + return prop.m_value; + } - static void set_value(VectorProperty &prop, - Value value, const int index) - { - prop.at(index) = value - .as() - .get(); - } + static const PropertyClass *get_object( + const VectorProperty &prop, const int index) + { + // By default, assume that ValueT is not a + throw runtime_error( + "Tried calling get_object() on a property that does not store an object." + ); + } - static const PropertyClass *get_object(const VectorProperty &prop, const int index) - { - // ValueT does not derive from PropertyClass, and so, this property is not - // storing an object. - throw runtime_error( - "Tried calling get_object() on a property that does not store an object." - ); - } + static void set_object(VectorProperty &prop, + std::unique_ptr &object, const int index) + { + throw runtime_error( + "Tried calling set_object() on a property that does not store an object." + ); + } + }; - static void set_object(VectorProperty &prop, - std::unique_ptr &object, const int index) + /** + * A helper utility that provides the right implementation of copy(), + * get_object() and set_object(), based on characteristics of type: ValueT. + */ + template < + typename ValueT, + typename Enable = void + > + struct vector_object_helper : default_vector_object_helper { - // ValueT does not derive from PropertyClass, and so, this property is not - // storing an object. - throw runtime_error( - "Tried calling set_object() on a property that does not store an object." - ); - } - }; + using default_vector_object_helper::copy; + using default_vector_object_helper::get_object; + using default_vector_object_helper::set_object; + }; + + /** + * Specialization for when ValueT is: + * - Not a pointer; but + * - does derive from PropertyClass + */ + template + struct vector_object_helper< + ValueT, + typename std::enable_if< + std::is_base_of::value + >::type + > + : default_vector_object_helper + { + using default_vector_object_helper::copy; + + static const PropertyClass *get_object( + const VectorProperty &prop, const int index) + { + // ValueT does derive from PropertyClass, and we have an instance of ValueT, + // so we can cast down to a PropertyClass pointer. + return dynamic_cast(&prop.at(index)); + } + + static void set_object(VectorProperty &prop, + std::unique_ptr &object, const int index) + { + // Ensure that object is not nullptr + if (!object) + throw runtime_error("Value cannot be null."); + + // Ensure that object is exactly the type of the property. + assert_type_match(prop.get_type(), object->get_type()); + + // ValueT does derive from PropertyClass, but we don't store a pointer, + // so we need to copy the value in. + prop.at(index) = *dynamic_cast(object.get()); + } + }; + + /** + * Specialization for when ValueT is: + * - A pointer; but + * - does not derive from PropertyClass + */ + template + struct vector_object_helper< + ValueT *, + typename std::enable_if< + !std::is_base_of::value + >::type + > + : pointer_vector_object_helper + { + using pointer_vector_object_helper::copy; + using pointer_vector_object_helper::get_object; + using pointer_vector_object_helper::set_object; + }; + + /** + * Specialization for when ValueT is: + * - A pointer; and + * - does derive from PropertyClass + */ + template + struct vector_object_helper< + ValueT *, + typename std::enable_if< + std::is_base_of::value + >::type + > + : pointer_vector_object_helper + { + using pointer_vector_object_helper::copy; + + static const PropertyClass *get_object( + const VectorProperty &prop, const int index) + { + // ValueT does derive from PropertyClass, and we have a pointer to an instance + // of ValueT, so we can cast down to a PropertyClass pointer. + return dynamic_cast(prop.at(index)); + } + + static void set_object(VectorProperty &prop, + std::unique_ptr &object, const int index) + { + // Ensure that object inherits the type of the property + if (object) + assert_type_match(prop.get_type(), object->get_type(), true); + + // ValueT does derive from PropertyClass, and we have a pointer to an instance + // of PropertyClass, so cast the pointer up to a ValueT. + prop.at(index) = dynamic_cast(object.release()); + } + }; + + /** + * A helper utility that provides the right implementation of + * get_value() and set_value() based on whether ValueT is a pointer + * for vector properties. + */ + template + struct vector_value_helper + { + static Value get_value(const VectorProperty &prop, const int index) + { + return Value::make_reference(prop.at(index)); + } + + static void set_value(VectorProperty &prop, + Value value, const int index) + { + Value casted_value = value.as(); + prop.at(index) = casted_value.get(); + } + }; + + /** + * Specialization for when ValueT is a pointer. + * + * Dereference the pointer before creating Value instances. + * This is so that the Value stores a pointer to a ValueT instance, + * rather than storing a pointer to a pointer. + */ + template + struct vector_value_helper + { + static Value get_value(const VectorProperty &prop, const int index) + { + ValueT *value_ptr = prop.at(index); + if (value_ptr == nullptr) + throw runtime_error("Called get_value() but value is nullptr."); + return Value::make_reference(*value_ptr); + } + + static void set_value(VectorProperty &prop, + Value value, const int index) + { + Value casted_value = value.as(); + prop.at(index) = casted_value.release(); + } + }; + } /** - * - */ - template - struct vector_value_object_helper< - ValueT, - typename std::enable_if< - std::is_pointer::value - >::type, - typename std::enable_if< - !std::is_base_of< - PropertyClass, - typename std::remove_pointer::type - >::value - >::type - > - { - using nonpointer_type = typename std::remove_pointer::type; - - static ValueT copy(VectorProperty &prop, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - // The copy constructor for all pointers is to copy the pointer - // without creating a new copy of the object it's pointing to. - return prop.at(index); - } - - static Value get_value(const VectorProperty &prop, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - return Value::make_reference(*prop.at(index)); - } - - static void set_value(VectorProperty &prop, - Value value, const int index) - { - prop.at(index) = value - .as() - .release(); - } - - static const PropertyClass *get_object(const VectorProperty &prop, const int index) - { - // ValueT does not derive from PropertyClass, and so, this property is not - // storing an object. - throw runtime_error( - "Tried calling get_object() on a property that does not store an object." - ); - } - - static void set_object(VectorProperty &prop, - std::unique_ptr &object, const int index) - { - // ValueT does not derive from PropertyClass, and so, this property is not - // storing an object. - throw runtime_error( - "Tried calling set_object() on a property that does not store an object." - ); - } - }; - - /** - * - */ - template - struct vector_value_object_helper< - ValueT, - typename std::enable_if< - std::is_pointer::value - >::type, - typename std::enable_if< - std::is_base_of< - PropertyClass, - typename std::remove_pointer::type - >::value - >::type - > - { - using nonpointer_type = typename std::remove_pointer::type; - - static ValueT copy(VectorProperty &prop, const int index) - { - // The copy constructor for all pointers is to copy the pointer - // without creating a new copy of the object it's pointing to. - return prop.at(index); - } - - static Value get_value(const VectorProperty &prop, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - return Value::make_reference(*prop.at(index)); - } - - static void set_value(VectorProperty &prop, - Value value, const int index) - { - prop.at(index) = value - .as() - .release(); - } - - static const PropertyClass *get_object(const VectorProperty &prop, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - // ValueT does derive from PropertyClass, and we have a pointer to an instance - // of ValueT, so we can cast down to a PropertyClass pointer. - return dynamic_cast(prop.at(index)); - } - - static void set_object(VectorProperty &prop, - std::unique_ptr &object, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - // Ensure that object inherits the type of the property - if (object) - assert_type_match(prop.get_type(), object->get_type(), true); - - // ValueT does derive from PropertyClass, and we have a pointer to an instance - // of PropertyClass, so cast the pointer up to a ValueT. - prop.at(index) = dynamic_cast(object.release()); - } - }; - - /** - * - */ - template - struct vector_value_object_helper< - ValueT, - typename std::enable_if< - !std::is_pointer::value - >::type, - typename std::enable_if< - std::is_base_of< - PropertyClass, - typename std::remove_pointer::type - >::value - >::type - > - { - static ValueT copy(VectorProperty &prop, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - // Derivitives of PropertyClass implement a clone method that returns - // a pointer to a copy. - ValueT *value_ptr = dynamic_cast(prop.at(index).copy()); - ValueT value = *value_ptr; - delete value_ptr; - return value; - } - - static Value get_value(const VectorProperty &prop, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - return Value::make_reference(prop.at(index)); - } - - static void set_value(VectorProperty &prop, - Value value, const int index) - { - prop.at(index) = value - .as() - .get(); - } - - static const PropertyClass *get_object(const VectorProperty &prop, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - // ValueT does derive from PropertyClass, and we have an instance of ValueT, - // so we can cast down to a PropertyClass pointer. - return dynamic_cast(&prop.at(index)); - } - - static void set_object(VectorProperty &prop, - std::unique_ptr &object, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - // Ensure that object is not nullptr - if (!object) - throw runtime_error("Value cannot be null."); - - // Ensure that object is exactly the type of the property - assert_type_match(prop.get_type(), object->get_type()); - - // ValueT does derive from PropertyClass, but we don't store a pointer, - // so we need to copy the value in. - prop.at(index) = *dynamic_cast(object.get()); - } - }; - - /** - * - */ - template < - typename ValueT, - typename IsPointerEnable = void - > - struct vector_value_rw_helper - { - static void write_value_to(const VectorProperty &prop, BitStream &stream, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - prop.get_type().write_to( - stream, - Value::make_reference(prop.at(index)) - ); - } - - static void read_value_from(VectorProperty &prop, BitStream &stream, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - Value value = prop.get_type().read_from(stream); - prop.at(index) = value.get(); - } - }; - - /** - * - */ - template - struct vector_value_rw_helper< - ValueT, - typename std::enable_if::value>::type - > - { - using type = typename std::remove_pointer::type; - - static void write_value_to(const VectorProperty &prop, BitStream &stream, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - prop.get_type().write_to( - stream, - Value::make_reference(*prop.at(index)) - ); - } - - static void read_value_from(VectorProperty &prop, BitStream &stream, const int index) - { - // Ensure index is within bounds - if (index < 0 || index >= prop.size()) - throw runtime_error("Index out of bounds."); - - Value value = prop.get_type().read_from(stream); - ValueT &value_ref = prop.at(index); - value_ref = value.release(); - } - }; - - /** - * - */ - template - struct vector_value_helper - { - static ValueT copy(VectorProperty &prop, const int index) - { - return vector_value_object_helper::copy(prop, index); - } - - static Value get_value(const VectorProperty &prop, - const int index) - { - return vector_value_object_helper::get_value(prop, index); - } - - static void set_value(VectorProperty &prop, - Value value, const int index) - { - vector_value_object_helper::set_value(prop, value, index); - } - - static const PropertyClass *get_object(const VectorProperty &prop, - const int index) - { - return vector_value_object_helper::get_object(prop, index); - } - - static void set_object(VectorProperty &prop, - std::unique_ptr &object, const int index) - { - vector_value_object_helper::set_object(prop, object, index); - } - - static void write_value_to(const VectorProperty &prop, - BitStream &stream, const int index) - { - vector_value_rw_helper::write_value_to(prop, stream, index); - } - - static void read_value_from(VectorProperty &prop, - BitStream &stream, const int index) - { - vector_value_rw_helper::read_value_from(prop, stream, index); - } - }; - /// @endcond - - /** - * + * A dynamically-sized array property. */ template class VectorProperty : public std::vector, public IProperty @@ -422,7 +261,7 @@ namespace pclass { // Copy vector values into this vector for (auto i = 0; i < this->size(); i++) - this->push_back(vector_value_helper::copy(*this, i)); + this->push_back(detail::vector_value_helper::copy(*this, i)); } constexpr bool is_pointer() const override @@ -454,34 +293,28 @@ namespace pclass { if (index < 0 || index >= this->size()) throw runtime_error("Index out of bounds."); - return vector_value_helper::get_value(*this, index); + return detail::vector_value_helper::get_value(*this, index); } void set_value(Value value, std::size_t index) override { if (index < 0 || index >= this->size()) throw runtime_error("Index out of bounds."); - return vector_value_helper::set_value(*this, value, index); + return detail::vector_value_helper::set_value(*this, value, index); } const PropertyClass *get_object(const std::size_t index) const override { - return vector_value_helper::get_object(*this, index); + if (index < 0 || index >= this->size()) + throw runtime_error("Index out of bounds."); + return detail::vector_object_helper::get_object(*this, index); } void set_object(std::unique_ptr &object, std::size_t index) override { - return vector_value_helper::set_object(*this, object, index); - } - - void write_value_to(BitStream &stream, const std::size_t index) const override - { - vector_value_helper::write_value_to(*this, stream, index); - } - - void read_value_from(BitStream &stream, const std::size_t index) override - { - vector_value_helper::read_value_from(*this, stream, index); + if (index < 0 || index >= this->size()) + throw runtime_error("Index out of bounds."); + return detail::vector_object_helper::set_object(*this, object, index); } }; } diff --git a/src/pclass/Property.cpp b/src/pclass/Property.cpp index 163bd46..3d5857e 100644 --- a/src/pclass/Property.cpp +++ b/src/pclass/Property.cpp @@ -80,5 +80,19 @@ namespace pclass { return 0; } + + void IProperty::write_value_to(BitStream& stream, const std::size_t index) const + { + if (index < 0 || index >= get_element_count()) + throw runtime_error("Index out of bounds."); + get_type().write_to(stream, get_value(index)); + } + + void IProperty::read_value_from(BitStream& stream, const std::size_t index) + { + if (index < 0 || index >= get_element_count()) + throw runtime_error("Index out of bounds."); + set_value(get_type().read_from(stream), index); + } } }