patterncppMinor
Reimplemented QAbstractTableModel::setData
Viewed 0 times
setdataqabstracttablemodelreimplemented
Problem
I have subclassed Qt
My humble opinion is that the code is very ugly. Can someone show me guidelines for its optimisation?
QAbstractTableModel with QJsonDocument as data source which I have reimplemented the setData() method in:bool UeJsonPlacesTableModel::setData(const QModelIndex& index,
const QVariant& value,
int role)
{
if(role!=Qt::EditRole||
index.row()=this->m_ueJsonData.isArray()?this->m_ueJsonData.array().size():this->m_ueJsonData.isObject()?this->m_ueJsonData.object().size():0||
index.column()=this->m_ueJsonData.isArray()?this->m_ueJsonData.array().size():this->m_ueJsonData.isObject()?this->m_ueJsonData.object().size()>0?this->m_ueJsonData.object().size():0:0)
{
return false;
} // if
QVariantList dataList=this->m_ueJsonData.toVariant().toList();
QVariantMap dataVariantMap=this->m_ueJsonData.toVariant().toList().at(index.row()).toMap();
QVariantMap::const_iterator dataIterator=dataVariantMap.constBegin();
int dataIndex=0;
QString keyName=QString();
QString dataValue=QString();
while(dataIterator!=dataVariantMap.constEnd())
{
if(dataIndex==index.column())
{
keyName=dataVariantMap.keys().at(dataIndex);
}
else
{
dataIterator++;
dataIndex++;
} // if
} // while
QVariantMap changedData;
changedData.insert(keyName,
value.toString());
dataList.replace(index.row(),
changedData);
this->m_ueJsonData=QJsonDocument::fromVariant(dataList);
emit(dataChanged(index,
index));
return true;
} // setDataMy humble opinion is that the code is very ugly. Can someone show me guidelines for its optimisation?
Solution
if(role!=Qt::EditRole||
index.row()=this->m_ueJsonData.isArray()?this->m_ueJsonData.array().size():this->m_ueJsonData.isObject()?this->m_ueJsonData.object().size():0||
index.column()=this->m_ueJsonData.isArray()?this->m_ueJsonData.array().size():this->m_ueJsonData.isObject()?this->m_ueJsonData.object().size()>0?this->m_ueJsonData.object().size():0:0)
{Stop. Regardless of what you're doing here, you should not be doing so many things in one condition. This looks like you need to write a new function instead.
Talking about functions, they'd greatly improve the readability and maintainability of your code. Now you got everything in one big
bool. Your while could use it's own function. The data changing, inserting and replacing could probably use a wrapper as well. If you give those functions meaningful names, the readability will increase big time!Imagine the following:
You haven't touched this code in 6 months. Now you want to add a feature. To add this feature, you try to understand how this code works and why you wrote it like this. Since it has been a while, you can't rely on your memory. How long would it take you to grasp the workings of this function?
Exactly. Way longer than necessary. Now imagine you share this code. Like you did here. How long does it take someone not familiar with your program to figure it out? Way longer than necessary.
For the sake of your future self, improve the readability of the code by splitting things up.
Code Snippets
if(role!=Qt::EditRole||
index.row()<0||
index.row()>=this->m_ueJsonData.isArray()?this->m_ueJsonData.array().size():this->m_ueJsonData.isObject()?this->m_ueJsonData.object().size():0||
index.column()<0||
index.column()>=this->m_ueJsonData.isArray()?this->m_ueJsonData.array().size():this->m_ueJsonData.isObject()?this->m_ueJsonData.object().size()>0?this->m_ueJsonData.object().size():0:0)
{Context
StackExchange Code Review Q#137958, answer score: 3
Revisions (0)
No revisions yet.