SidebarModel: Clean up variable naming and use C++ casts #3922
Conversation
This adds the `p` prefix to pointer variables, uses camelCase naming consistently and replaces C-style typecasts with C++ casts.
|
|
||
| // `this` is const, but the function expects a non-const pointer. | ||
| // TODO: Check if we can get rid of this const cast somehow. | ||
| return createIndex(row, column, const_cast<SidebarModel*>(this)); |
There was a problem hiding this comment.
This is IMHO one of the rare cases where a const cast is correct.
The function itself is const, and it returns an object with a pointer to itself.
Nothing changes in the object before the function is done. So the const nature is maintained.
Interestingly this function takes a const pointer in Qt 6 here:
https://github.com/qt/qtbase/blob/9db7cc79a26ced4997277b5c206ca15949133240/src/corelib/itemmodels/qabstractitemmodel.h#L404
I would prefer to cast to the parameter type directly:```suggestion
return createIndex(row, column, const_cast<void*>(this));
There was a problem hiding this comment.
Taking the this pointer is redundant by the way:
https://github.com/qt/qtbase/blob/9db7cc79a26ced4997277b5c206ca15949133240/src/corelib/itemmodels/qabstractitemmodel.h#L457
We can access it by QModelIndex::model()
However there is a warning
A const pointer to the model is returned because calls to non-const functions of the model might invalidate the model index and possibly crash your application.
That is what we do now ... :-/
There was a problem hiding this comment.
Do you have interest to check, if we can just replace internalPointer() by model() and keep the constnes of the pointer?
There was a problem hiding this comment.
Do you have interest to check, if we can just replace internalPointer() by model() and keep the constnes of the pointer?
I don't really get what you mean. Can you give me a code example?
This adds the
pprefix to pointer variables, uses camelCase namingconsistently and replaces C-style typecasts with C++ casts.
We'll need some refactoring of the library handling to support QML, so I thought it would be best to take care of this separately.
I also found two issues where we cast a const pointer to a mutable one. That was hidden by C-style typecast. I didn't fix it here, just make the issue obvious by using
const_cast.