Skip to content

Commit

Permalink
created ExtendedProperties for BodyNode
Browse files Browse the repository at this point in the history
  • Loading branch information
mxgrey committed Aug 5, 2015
1 parent 84170f2 commit cbde581
Show file tree
Hide file tree
Showing 9 changed files with 458 additions and 166 deletions.
48 changes: 2 additions & 46 deletions dart/common/AddonManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,55 +79,11 @@ class AddonManager
{
public:

/// MapHolder is a templated wrapper class that is used to allow maps of
/// Addon::State and Addon::Properties to be handled in a semantically
/// palatable way.
template <typename MapType>
class MapHolder final
{
public:

/// Default constructor
MapHolder() = default;

/// Copy constructor
MapHolder(const MapHolder& otherStates);

/// Move constructor
MapHolder(MapHolder&& otherStates);

/// Map-based constructor
MapHolder(const MapType& otherMap);

/// Map-based move constructor
MapHolder(MapType&& otherMap);

/// Assignment operator
MapHolder& operator=(const MapHolder& otherStates);

/// Move assignment operator
MapHolder& operator=(MapHolder&& otherStates);

/// Map-based assignment operator
MapHolder& operator=(const MapType& otherMap);

/// Map-based move assignment operator
MapHolder& operator=(MapType&& otherMap);

/// Get the map of Addon::States
const MapType& getMap() const;

private:

/// A map containing the collection of States for the Addon
MapType mMap;
};

using StateMap = std::map< std::type_index, std::unique_ptr<Addon::State> >;
using State = MapHolder<StateMap>;
using State = ExtensibleMapHolder<StateMap>;

using PropertiesMap = std::map< std::type_index, std::unique_ptr<Addon::Properties> >;
using Properties = MapHolder<PropertiesMap>;
using Properties = ExtensibleMapHolder<PropertiesMap>;

/// Virtual destructor
virtual ~AddonManager() = default;
Expand Down
88 changes: 87 additions & 1 deletion dart/common/Extensible.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#define DART_COMMON_EXTENSIBLE_H_

#include <memory>
#include <vector>

namespace dart {
namespace common {
Expand Down Expand Up @@ -69,11 +70,96 @@ class Extensible
/// Implement this function to allow your Extensible type to be copied safely.
virtual std::unique_ptr<T> clone() const = 0;

/// Copy the contents of anotherExtensible into this one
/// Copy the contents of anotherExtensible into this one. We do not enforce
/// that the incoming type is const, but we trust you not to modify its
/// contents.
virtual void copy(const T& anotherExtensible) = 0;

This comment has been minimized.

Copy link
@jslee02

jslee02 Oct 14, 2015

Member

It seems like the const types of what the comment says and the argument are different. Remove const from the argument?

Beside on the mismatch, why don't we want to enforce that the incoming type is const?

This comment has been minimized.

Copy link
@mxgrey

mxgrey Oct 15, 2015

Author Member

I think that comment is either outdated or in the wrong place.

I vaguely remember creating a copy function that took in a const std::unique_ptr<T>& argument, in which case the unique_ptr was const but the T was not, and I think this comment was meant to refer to that. If we were to pass in a const std::unique_ptr<T>& here, that would allow the function to modify the contents of the incoming data, which would be very risky. Unlike with shared_ptr, I don't think there's a way to convert a std::unique_ptr<T> to a std::unique_ptr<const T> (please tell me if I'm wrong about this), so instead I decided to pass in a const dereferenced version when copying.

So I should just remove that comment, since I'm pretty sure it's outdated.

This comment has been minimized.

Copy link
@jslee02

jslee02 Oct 16, 2015

Member

I don't think there's a way to convert a std::unique_ptr to a std::unique_ptr (please tell me if I'm wrong about this), so instead I decided to pass in a const dereferenced version when copying.

👍 I tried to find a way but couldn't. I found a post of similar issue with this, and your way was suggested there too.

This comment has been minimized.

Copy link
@mkoval

mkoval Oct 16, 2015

Collaborator

👍 For "passing in a const dereferenced version when copying."

I don't see any reason for copy to take a unique_ptr, anyway. It really just needs the value it contains.

};

/// MapHolder is a templated wrapper class that is used to allow maps of
/// Addon::State and Addon::Properties to be handled in a semantically
/// palatable way.
template <typename MapType>
class ExtensibleMapHolder final
{
public:

/// Default constructor
ExtensibleMapHolder() = default;

/// Copy constructor
ExtensibleMapHolder(const ExtensibleMapHolder& otherStates);

/// Move constructor
ExtensibleMapHolder(ExtensibleMapHolder&& otherStates);

/// Map-based constructor
ExtensibleMapHolder(const MapType& otherMap);

/// Map-based move constructor
ExtensibleMapHolder(MapType&& otherMap);

/// Assignment operator
ExtensibleMapHolder& operator=(const ExtensibleMapHolder& otherStates);

/// Move assignment operator
ExtensibleMapHolder& operator=(ExtensibleMapHolder&& otherStates);

/// Map-based assignment operator
ExtensibleMapHolder& operator=(const MapType& otherMap);

/// Map-based move assignment operator
ExtensibleMapHolder& operator=(MapType&& otherMap);

/// Get the map of Addon::States
const MapType& getMap() const;

private:

/// A map containing the collection of States for the Addon
MapType mMap;
};

/// The ExtensibleVector type wraps a std::vector of an Extensible type allowing
/// it to be handled by an ExtensibleMapHolder
template <typename T>
class ExtensibleVector final
{
public:

/// Default constructor
ExtensibleVector() = default;

/// Construct from a regular vector
ExtensibleVector(const std::vector<T>& regularVector);

/// Construct from a regular vector using move semantics
ExtensibleVector(std::vector<T>&& regularVector);

/// Do not copy this class directly, use clone() or copy() instead
ExtensibleVector(const ExtensibleVector& doNotCopy) = delete;

/// Do not copy this class directly, use clone() or copy() instead
ExtensibleVector& operator=(const ExtensibleVector& doNotCopy) = delete;

/// Create a copy of this ExtensibleVector's contents
std::unique_ptr< ExtensibleVector<T> > clone() const;

/// Copy the contents of another extensible vector into this one.
void copy(const ExtensibleVector<T>& anotherVector);

/// Get a reference to the std::vector that this class is wrapping
const std::vector<T>& getVector() const;

private:

/// The std::vector that this class is wrapping
std::vector<T> mVector;
};

} // namespace common
} // namespace dart

#include "dart/common/detail/Extensible.h"

#endif // DART_COMMON_EXTENSIBLE_H_
116 changes: 0 additions & 116 deletions dart/common/detail/AddonManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,122 +42,6 @@
namespace dart {
namespace common {

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>::MapHolder(const MapHolder& otherHolder)
{
*this = otherHolder;
}

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>::MapHolder(MapHolder&& otherHolder)
{
*this = std::move(otherHolder);
}

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>::MapHolder(const MapType& otherMap)
{
*this = otherMap;
}

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>::MapHolder(MapType&& otherMap)
{
*this = std::move(otherMap);
}

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>& AddonManager::MapHolder<MapType>::operator=(
const MapHolder& otherHolder)
{
*this = otherHolder.getMap();

return *this;
}

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>& AddonManager::MapHolder<MapType>::operator=(
MapHolder&& otherHolder)
{
mMap = std::move(otherHolder.mMap);

return *this;
}

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>& AddonManager::MapHolder<MapType>::operator=(
const MapType& otherMap)
{
typename MapType::iterator receiver = mMap.begin();
typename MapType::const_iterator sender = otherMap.begin();

while( otherMap.end() != sender )
{
if( mMap.end() == receiver )
{
// If we've reached the end of this MapHolder's map, then we should just
// add each entry
mMap[sender->first] = sender->second->clone();
++sender;
}
else if( receiver->first == sender->first )
{
// We should copy the incoming object when possible so we can avoid the
// memory allocation overhead of cloning.
if(receiver->second)
receiver->second->copy(*sender->second);
else
receiver->second = sender->second->clone();

++receiver;
++sender;
}
else if( receiver->first < sender->first )
{
// Clear this entry in the map, because it does not have an analog in the
// map that we are copying
receiver->second = nullptr;
++receiver;
}
else
{
mMap[sender->first] = sender->second->clone();
++sender;
}
}

while( mMap.end() != receiver )
{
mMap.erase(receiver++);
}

return *this;
}

//==============================================================================
template <typename MapType>
AddonManager::MapHolder<MapType>& AddonManager::MapHolder<MapType>::operator=(
MapType&& otherHolder)
{
mMap = std::move(otherHolder);

return *this;
}

//==============================================================================
template <typename MapType>
const MapType& AddonManager::MapHolder<MapType>::getMap() const
{
return mMap;
}

//==============================================================================
template <class T>
bool AddonManager::has() const
Expand Down
Loading

0 comments on commit cbde581

Please sign in to comment.