Refactoring in NoteListView #212
Conversation
| @@ -584,7 +477,7 @@ void NoteListView::onSelectFirstNoteEvent() | |||
|
|
|||
| const QAbstractItemModel * pModel = model(); | |||
| if (Q_UNLIKELY(!pModel)) { | |||
| QNDEBUG(QStringLiteral("No model")); | |||
| QNERROR(QStringLiteral("No model")); | |||
There was a problem hiding this comment.
Messages printed to error log level better be more verbose. Sure, it's possible to look at the source file and line number to figure out where such log entry comes from but it'd be more convenient if such error message was pretty clear on its own, like "No model is set to NoteListView on select first note event".
There was a problem hiding this comment.
Yes agree. Just in this particular case, I think its very unlikely the model to be null. And if this happen, then something must be really terribly bad (like design error or like this).
But on general case you are right.
No problem with improving the message.
|
Thanks, merged this! FYI, I chose to do some adjustments after the merge: I try to not use Nevertheless, I think this PR has made a really good progress on the refactoring front, so please keep it up! |
I agree, that it may be less readable. Actually it was suggested by IDE :) |
Preparation for #197
Its the refactoring we discussed.
I tried to incorporate your comments and name the method according to Qt best practices.
It isn't that nice as I wanted it to have :) but still should be improvement.
There should be not functionality changes (except the error messages are more general, but this should not matter; in case of "unlikely" error in this shortcut function - things will be already quite bad).