Inconsistent semantics between LoanedSample<T> and Sample<T>

3 posts / 0 new
Log in or register to post comments
Last post
Mon, 01/29/2018 - 00:30
#1
bitbucket
Offline
Last seen: 5 years 7 months ago
Joined: 10/29/2015
Posts: 12
Inconsistent semantics between LoanedSample<T> and Sample<T>

I was trying to copy some LoanedSamples I had into "non-loaned" dds::sub::Samples. From the documentation, I would expect that I can always convert an rti::sub::LoanedSample<T> into an equivalent dds::sub::Sample<T>. However, it turns out that the constructor, Sample<T>::Sample(const rti::sub::LoanedSample<T>&), does not check whether the LoanedSample is valid or not, and simply tries to access the data() field. From include/ndds/hpp/rti/sub/SampleImpl.hpp:

    SampleImpl(const SampleImpl& other)
        : data_(other.data()), 
          info_(other.info())
    {
    }

I cannot really think of a valid reason why it should behave like this; at the very least there needs to be a big warning in the documentation that you cannot use this constructor on invalid samples.

Now, I can of course build invalid Sample<T> objects without any problem. In fact the default constructor does just that, it constructs an invalid sample. And that brings me to another issue: when calling Sample<T>::data() on an invalid sample, it does not throw an exception, but lets you access the (usually default-constructed) data without complaining.

I would say that the first issue – the exception when constructing from an invalid LoanedSample – is definitely a bug. The second issue – accessing data() on an invalid LoanedSample() without an exception – might not qualify as a bug, it could be intended behavior. But in that case I would expect the documentation to make me aware of this.

Are these differences between LoanedSample and Sample a bug, an oversight, or a slightly underdocumented feature?

This is with RTI Connext 5.2.0. Do newer versions differ in this regard?

Thanks for any information.

Top
Tue, 02/06/2018 - 18:48
alexc
Offline
Last seen: 2 weeks 2 days ago
Joined: 04/02/2013
Posts: 195

Hi,

I agree with your comments. Sample has both an inconsitent and underdocumented behavior. I have filed a bug to fix the constructor and not throw an exception when creating a Sample from a LoanedSample with invalid data (reference: CORE-8446) and a documentation bug to clarify how Sample::data() should work (reference: CORE-8447).

I'm not sure how you're copying LoanedSamples into Samples, but if you want to skip invalid-data samples, you can use the iterator adapter rti::sub::valid_samples. For example, given a LoanedSamples container 'samples' and a destination 'data_vector' (can be a std::vector<Foo> or std::vector<Sample<Foo>>) :

std::copy(
  rti::sub::valid_samples(samples.begin()),
  rti::sub::valid_samples(samples.end()),
  std::back_inserter(data_vector));

I hope this helps,

Alex Campos

Principal Software Engineer, RTI

 

Top
Wed, 02/07/2018 - 02:37
bitbucket
Offline
Last seen: 5 years 7 months ago
Joined: 10/29/2015
Posts: 12

Alex,

thanks very much for your help! In my use case, I want to convert all samples, valid or not, from Loaned to non-Loaned. I use a small helper function:

template <typename T>
dds::sub::Sample<T> sampleFromLoaned(const rti::sub::LoanedSample<T>& loaned)
{
  if (loaned.info().valid())
  {
    return dds::sub::Sample<T>(loaned);
  }
  else
  {
    dds::sub::Sample<T> s;
    s.info(loaned.info());
    return s;
  }
}

Copying a whole set of samples is done with a simple range-based for loop:

std::vector<dds::sub::Sample<T>> non_loaned_samples;
non_loaned_samples.reserve(samples.length());
for (const auto& sample : samples)
{
  non_loaned_samples.emplace_back(sampleFromLoaned(sample));
}
Top