Skip to content

Commit

Permalink
[SofaCore] FIX SingleLink clear/set behavior (#1749)
Browse files Browse the repository at this point in the history
* [SofaCore] Add few tests showing what should be the expected behavior of SingleLink

At this step the tests are failing.

* [SofaCore] Fix SingleLink behavior regarding clear()/set()

* [WIP]
  • Loading branch information
damienmarchal authored Jan 28, 2021
1 parent f65b3b2 commit 5006ffc
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class EmptyObject : public BaseObject
class SingleLink_test: public BaseTest
{
public:
SingleLink<BaseObject, BaseObject, BaseLink::FLAG_DOUBLELINK|BaseLink::FLAG_STRONGLINK > m_link ;
SingleLink<BaseObject, BaseObject, BaseLink::FLAG_DOUBLELINK|BaseLink::FLAG_STRONGLINK|BaseLink::FLAG_STOREPATH > m_link ;
BaseObject::SPtr m_dst ;
BaseObject::SPtr m_src ;

Expand Down Expand Up @@ -116,3 +116,29 @@ TEST_F(SingleLink_test, getOwnerBase)
m_link.setOwner(aBaseObject.get());
ASSERT_EQ(m_link.getOwnerBase(), aBaseObject.get());
}

TEST_F(SingleLink_test, checkClearSetValue )
{
m_link.clear();
ASSERT_EQ( m_link.size(), 0 ) << "The size of a link container should be zero after clear().";
m_link.set(nullptr);
ASSERT_EQ( m_link.size(), 1 ) << "The size of a link container should be one after set(nullptr).";
ASSERT_EQ( m_link.getLinkedPath(), "" ) << "The path should be empty because of the the previously used set(nullptr).";
}

TEST_F(SingleLink_test, checkClearSetPath )
{
m_link.clear();
ASSERT_EQ( m_link.size(), 0 ) << "The size of a link container should be zero after clear().";
m_link.setPath("@/ThisIsAPath");
ASSERT_EQ( m_link.size(), 1 ) << "The size of a link container should be one after setPath().";
ASSERT_EQ( m_link.getLinkedPath(), "@/ThisIsAPath" ) << "The path should not be empty as it was set previously.";
}

TEST_F(SingleLink_test, checkEmptyness )
{
m_link.set(nullptr);
ASSERT_EQ( m_link.size(), 1 );
m_link.clear();
ASSERT_EQ( m_link.size(), 0 );
}
37 changes: 31 additions & 6 deletions SofaKernel/modules/SofaCore/src/sofa/core/objectmodel/Link.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,11 @@ class LinkTraitsContainer;


/// Class to hold 0-or-1 pointer. The interface is similar to std::vector (size/[]/begin/end), plus an automatic convertion to one pointer.
template < class T, class TPtr = T* >
template < class T, class TDestPtr, class TPtr = T* >
class SinglePtr
{
protected:
bool isEmpty {true};
TPtr elems[1];
public:
typedef T pointed_type;
Expand All @@ -135,7 +136,7 @@ class SinglePtr
}
const_iterator end() const
{
return (!elems[0])?elems:elems+1;
return (isEmpty)?elems:elems+1;
}
const_reverse_iterator rbegin() const
{
Expand Down Expand Up @@ -163,14 +164,22 @@ class SinglePtr
}
std::size_t size() const
{
return (!elems[0])?0:1;
return !isEmpty;
}
void resize(size_t size)
{
if(size == 1)
isEmpty=false;
else
isEmpty=true;
}
bool empty() const
{
return !elems[0];
return isEmpty;
}
void clear()
{
isEmpty = true;
elems[0] = TPtr();
}
const TPtr& get() const
Expand All @@ -181,6 +190,11 @@ class SinglePtr
{
return elems[0];
}
void add(TDestPtr v)
{
isEmpty = false;
elems[0] = v;
}
const TPtr& operator[](std::size_t i) const
{
return elems[i];
Expand Down Expand Up @@ -211,14 +225,18 @@ template<class TDestType, class TDestPtr, class TValueType>
class LinkTraitsContainer<TDestType, TDestPtr, TValueType, false>
{
public:
typedef SinglePtr<TDestType, TValueType> T;
typedef SinglePtr<TDestType, TDestPtr, TValueType> T;
static void resize(T& c, size_t newsize)
{
c.resize(newsize);
}
static void clear(T& c)
{
c.clear();
}
static std::size_t add(T& c, TDestPtr v)
{
c.get() = v;
c.add(v);
return 0;
}
static std::size_t find(const T& c, TDestPtr v)
Expand Down Expand Up @@ -352,6 +370,10 @@ class TLink : public BaseLink
return m_value.crend();
}
SOFA_END_DEPRECATION_AS_ERROR
void clear()
{
TraitsContainer::clear(m_value);
}

bool add(DestPtr v)
{
Expand Down Expand Up @@ -793,6 +815,7 @@ class SingleLink : public TLink<TOwnerType,TDestType,TFlags&~BaseLink::FLAG_MULT

void set(DestPtr v)
{
TraitsContainer::resize(m_value, 1);
ValueType& value = m_value.get();
const DestPtr before = TraitsValueType::get(value);
if (v == before) return;
Expand All @@ -803,6 +826,7 @@ class SingleLink : public TLink<TOwnerType,TDestType,TFlags&~BaseLink::FLAG_MULT

void set(DestPtr v, const std::string& path)
{
TraitsContainer::resize(m_value, 1);
ValueType& value = m_value.get();
const DestPtr before = TraitsValueType::get(value);
if (v != before)
Expand All @@ -815,6 +839,7 @@ class SingleLink : public TLink<TOwnerType,TDestType,TFlags&~BaseLink::FLAG_MULT

void setPath(const std::string& path)
{
TraitsContainer::resize(m_value, 1);
if (path.empty())
{
set(nullptr);
Expand Down

0 comments on commit 5006ffc

Please sign in to comment.