Skip to content

Commit

Permalink
TestSuite: don't go through the container twice in Compare::Container.
Browse files Browse the repository at this point in the history
Since this is a failure case, it doesn't really matter if it takes 10x
more time at runtime. But it definitely adds more work for the compiler,
and that's where most of human time is usually spent -- iterating on the
test code. So don't.
  • Loading branch information
mosra committed Jun 1, 2022
1 parent ca98065 commit ac3ad3e
Showing 1 changed file with 25 additions and 21 deletions.
46 changes: 25 additions & 21 deletions src/Corrade/TestSuite/Compare/Container.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,22 +69,33 @@ template<class T> class Comparator<Compare::Container<T>> {
private:
const T* _actualContents;
const T* _expectedContents;
std::size_t _firstDifferent;
};

template<class T> ComparisonStatusFlags Comparator<Compare::Container<T>>::operator()(const T& actual, const T& expected) {
_actualContents = &actual;
_expectedContents = &expected;

ComparisonStatusFlags status;
if(_actualContents->size() != _expectedContents->size())
return ComparisonStatusFlag::Failed;
status = ComparisonStatusFlag::Failed;

/* Recursively use comparator on the values */
/* Recursively use the comparator on the values, find the first different
item in the common prefix. If there's none, then the first different
item is right after the common prefix, and if both have the same size
then it means the containers are the same. */
Comparator<typename std::decay<decltype((*_actualContents)[0])>::type> comparator;
for(std::size_t i = 0; i != _actualContents->size(); ++i)
if(comparator((*_actualContents)[i], (*_expectedContents)[i]) & ComparisonStatusFlag::Failed)
return ComparisonStatusFlag::Failed;
const std::size_t commonPrefixSize = Utility::min(_actualContents->size(), _expectedContents->size());
_firstDifferent = commonPrefixSize;
for(std::size_t i = 0; i != commonPrefixSize; ++i) {
if(comparator((*_actualContents)[i], (*_expectedContents)[i]) & ComparisonStatusFlag::Failed) {
_firstDifferent = i;
status = ComparisonStatusFlag::Failed;
break;
}
}

return {};
return status;
}

template<class T> void Comparator<Compare::Container<T>>::printMessage(ComparisonStatusFlags, Utility::Debug& out, const char* actual, const char* expected) const {
Expand All @@ -96,21 +107,14 @@ template<class T> void Comparator<Compare::Container<T>>::printMessage(Compariso

out << *_actualContents << Utility::Debug::newline << " but expected\n " << *_expectedContents << Utility::Debug::newline << " ";

Comparator<typename std::decay<decltype((*_actualContents)[0])>::type> comparator;
for(std::size_t i = 0, end = Utility::max(_actualContents->size(), _expectedContents->size()); i != end; ++i) {
if(_actualContents->size() > i && _expectedContents->size() > i &&
!(comparator((*_actualContents)[i], (*_expectedContents)[i]) & ComparisonStatusFlag::Failed)) continue;

if(_actualContents->size() <= i)
out << "Expected has" << (*_expectedContents)[i];
else if(_expectedContents->size() <= i)
out << "Actual has" << (*_actualContents)[i];
else
out << "Actual" << (*_actualContents)[i] << "but" << (*_expectedContents)[i] << "expected";

out << "on position" << i << Utility::Debug::nospace << ".";
break;
}
if(_actualContents->size() <= _firstDifferent)
out << "Expected has" << (*_expectedContents)[_firstDifferent];
else if(_expectedContents->size() <= _firstDifferent)
out << "Actual has" << (*_actualContents)[_firstDifferent];
else
out << "Actual" << (*_actualContents)[_firstDifferent] << "but" << (*_expectedContents)[_firstDifferent] << "expected";

out << "on position" << _firstDifferent << Utility::Debug::nospace << ".";
}
#endif

Expand Down

0 comments on commit ac3ad3e

Please sign in to comment.