Ref T529, deterministic sort order when column values are equal

* the Qt model sorts by column
* when multiple values have the same column value the order among those is more or less random
* added additional property indexes to determine the order among the equal values
This commit is contained in:
Klaus Basan
2019-01-30 16:37:32 +01:00
committed by Mat Sutcliffe
parent 17f67d6106
commit 8c15f45007
8 changed files with 50 additions and 15 deletions

View File

@@ -127,8 +127,8 @@ namespace BlackGui
if (!formatter) { return false; }
ObjectType obj = m_container[index.row()];
ObjectType currentObject(obj);
CPropertyIndex propertyIndex = this->columnToPropertyIndex(index.column());
const ObjectType currentObject(obj);
const CPropertyIndex propertyIndex = this->columnToPropertyIndex(index.column());
obj.setPropertyByIndex(propertyIndex, value);
if (obj != currentObject)
@@ -149,7 +149,7 @@ namespace BlackGui
bool CListModelBase<T, UseCompare>::setInContainer(const QModelIndex &index, const ObjectType &obj)
{
if (!index.isValid()) { return false; }
int row = index.row();
const int row = index.row();
if (row < 0 || row >= this->container().size()) { return false; }
m_container[row] = obj;
return true;
@@ -215,8 +215,8 @@ namespace BlackGui
{
Q_UNUSED(sort);
if (m_modelDestroyed) { return nullptr; }
auto sortColumn = this->getSortColumn();
auto sortOrder = this->getSortOrder();
const auto sortColumn = this->getSortColumn();
const auto sortOrder = this->getSortOrder();
CWorker *worker = CWorker::fromTask(this, "ModelSort", [this, container, sortColumn, sortOrder]()
{
return this->sortContainerByColumn(container, sortColumn, sortOrder);
@@ -399,11 +399,11 @@ namespace BlackGui
template <typename T, bool UseCompare>
void CListModelBase<T, UseCompare>::remove(const ObjectType &object)
{
int oldSize = m_container.size();
const int oldSize = m_container.size();
beginRemoveRows(QModelIndex(), 0, 0);
m_container.remove(object);
endRemoveRows();
int newSize = m_container.size();
const int newSize = m_container.size();
if (oldSize != newSize)
{
this->emitModelDataChanged();
@@ -533,10 +533,10 @@ namespace BlackGui
}
// sort the values
std::integral_constant<bool, UseCompare> marker {};
const std::integral_constant<bool, UseCompare> marker {};
const auto p = [ = ](const ObjectType & a, const ObjectType & b) -> bool
{
return Private::compareForModelSort<ObjectType>(a, b, order, propertyIndex, marker);
return Private::compareForModelSort<ObjectType>(a, b, order, propertyIndex, m_sortTieBreakers, marker);
};
return container.sorted(p);
@@ -555,7 +555,7 @@ namespace BlackGui
for (const QModelIndex &index : indexes)
{
if (!index.isValid()) { continue; }
int r = index.row();
const int r = index.row();
if (rows.contains(r)) { continue; }
container.push_back(this->at(index));
rows.append(r);

View File

@@ -193,19 +193,32 @@ namespace BlackGui
{
//! Sort with compare function
template<class ObjectType>
bool compareForModelSort(const ObjectType &a, const ObjectType &b, Qt::SortOrder order, const BlackMisc::CPropertyIndex &index, std::true_type)
bool compareForModelSort(const ObjectType &a, const ObjectType &b, Qt::SortOrder order, const BlackMisc::CPropertyIndex &index, const BlackMisc::CPropertyIndexList &tieBreakers, std::true_type)
{
const int c = a.comparePropertyByIndex(index, b);
if (c == 0) { return false; }
if (c == 0)
{
if (!tieBreakers.isEmpty())
{
const std::integral_constant<bool, true> marker;
return compareForModelSort<ObjectType>(a, b, order, tieBreakers.front(), tieBreakers.copyFrontRemoved(), marker);
}
return false;
}
return (order == Qt::AscendingOrder) ? (c < 0) : (c > 0);
}
//! Sort without compare function
template <typename ObjectType>
bool compareForModelSort(const ObjectType &a, const ObjectType &b, Qt::SortOrder order, const BlackMisc::CPropertyIndex &index, std::false_type)
bool compareForModelSort(const ObjectType &a, const ObjectType &b, Qt::SortOrder order, const BlackMisc::CPropertyIndex &index, const BlackMisc::CPropertyIndexList &tieBreakers, std::false_type)
{
const BlackMisc::CVariant aQv = a.propertyByIndex(index);
const BlackMisc::CVariant bQv = b.propertyByIndex(index);
if (!tieBreakers.isEmpty() && aQv == bQv)
{
const std::integral_constant<bool, false> marker;
return compareForModelSort<ObjectType>(a, b, order, tieBreakers.front(), tieBreakers.copyFrontRemoved(), marker);
}
return (order == Qt::AscendingOrder) ? (aQv < bQv) : (bQv < aQv);
}
} // namespace

View File

@@ -163,6 +163,7 @@ namespace BlackGui
bool m_modelDestroyed = false; //!< model is about to be destroyed
Qt::SortOrder m_sortOrder; //!< sort order (asc/desc)
Qt::DropActions m_dropActions = Qt::IgnoreAction; //!< drop actions
BlackMisc::CPropertyIndexList m_sortTieBreakers; //!< how column values are sorted if equal, if no value is given this is random
private:
BlackMisc::CDigestSignal m_dsModelsChanged { this, &CListModelBaseNonTemplate::changed, &CListModelBaseNonTemplate::onChangedDigest, 500, 10 };

View File

@@ -40,8 +40,10 @@ namespace BlackGui
CListModelDbObjects<T, K, UseCompare>::CListModelDbObjects(const QString &translationContext, QObject *parent) :
CListModelBase<ContainerType, UseCompare>(translationContext, parent)
{
CListModelBaseNonTemplate::m_sortTieBreakers.push_front(ObjectType::keyIndex());
constexpr bool hasIntegerKey = std::is_base_of<IDatastoreObjectWithIntegerKey, ObjectType>::value && std::is_same<int, KeyType>::value;
constexpr bool hasStringKey = std::is_base_of<IDatastoreObjectWithStringKey, ObjectType>::value && std::is_base_of<QString, KeyType>::value;
constexpr bool hasStringKey = std::is_base_of<IDatastoreObjectWithStringKey, ObjectType>::value && std::is_base_of<QString, KeyType>::value;
static_assert(hasIntegerKey || hasStringKey, "ObjectType needs to implement IDatastoreObjectWithXXXXKey and have appropriate KeyType");
}
@@ -49,7 +51,7 @@ namespace BlackGui
QVariant CListModelDbObjects<T, K, UseCompare>::data(const QModelIndex &index, int role) const
{
if (role != Qt::BackgroundRole) { return CListModelBase<ContainerType, UseCompare>::data(index, role); }
if (isHighlightedIndex(index) ) { return QBrush(m_highlightColor); }
if (isHighlightedIndex(index)) { return QBrush(m_highlightColor); }
return CListModelBase<ContainerType, UseCompare>::data(index, role);
}

View File

@@ -26,6 +26,8 @@ namespace BlackGui
CListModelTimestampObjects<CStatusMessageList, true>("ViewStatusMessageList", parent)
{
this->setMode(Detailed);
m_sortTieBreakers.push_front(CStatusMessage::IndexMessage);
m_sortTieBreakers.push_front(CStatusMessage::IndexSeverity);
// force strings for translation in resource files
(void)QT_TRANSLATE_NOOP("ViewStatusMessageList", "time");