Code cleanup. #36
Code cleanup. #36
Conversation
Added const's and referances if needed.
| @@ -832,7 +832,7 @@ int MixxxApp::noOutputDlg(bool *continueClicked) | |||
| } | |||
| } | |||
|
|
|||
| QString buildWhatsThis(QString title, QString text) { | |||
| QString buildWhatsThis(QString& title, const QString& text) { | |||
There was a problem hiding this comment.
Probably yes. buildWhatsThis must not change its parameters, but there is title.replace(...) inside.
As for me, it would be more clear -- to make all args const and add temporary internal variable inside buildWhatsThis.
So, we can make all `buildWhatsThis`'s parameters `const`.
|
Done. Added temporary internal variable inside |
| QString buildWhatsThis(QString title, QString text) { | ||
| return QString("%1\n\n%2").arg(title.replace("&", ""), text); | ||
| QString buildWhatsThis(const QString& title, const QString& text) { | ||
| QString preparedTitle = title; |
There was a problem hiding this comment.
OK, if you like the reference parameters that much.
QString buildWhatsThis(QString prepareTitle, const QString& text)Would also just fine.
It is not worth argue, but I wander which code is more effective. Maybe almost the same.
There was a problem hiding this comment.
Ok, I wanted to know it, here are the results:
striped binary:
7.389.584 byte value version
7.385.488 byte reference version
while the function function itself is slightly longer with the reference.
But I was not able to find out why in the optimized dissembler.
There was a problem hiding this comment.
Sorry, but I can't get your idea. Can you explain what are you trying to check?
There was a problem hiding this comment.
First of all: This issue does not matter on a PC.
But I am an original embedded developer so I should now such things.
Here my theory, please correct me if I am wrong.
In the value parameter version the string copy on the stack is generated by the calling function and the address to this copy is passed.
In the reference parameter version the caller passes the address to the original string. The string copy is done in the called function later.
Unfortunately I cannot track these in the optimized assembler code. I think there are things like "copy on demand" going on.
But anyway, your final version produces less code. I thing this must be because the copy code is only one time in the binary instead of each time the function is called. I guess from the performance point of view both versions are almost the same because a copy must be done in any case.
The difference is such big, because the ext4 uses 4 KiB pages. We must have a look at the map file to see the truth.
|
Thank you @troyane! |
fix incorrect cue quantization with prevBeat
Vinyl control revisions
release Mixxx 2.1.4
Added const's and referances if needed.