Le 07/10/2020 à 16:31, Yuriy Skalko a écrit :
And the last patch based on the static analyzers output.
I like that. I have to admit that I gave up checking that the loops were
equivalent roughly at the first third of the patch :)
Is it done by hand or with some tool?
A few remarks, first on this archetypal example:
- for (int i = 0; i < nb_simplefeatures; ++i) {
- if (mustProvide(simplefeatures[i]))
- packages << "\\usepackage{" << simplefeatures[i] <<
"}\n";
+ for (auto & feature : simplefeatures) {
+ if (mustProvide(feature))
+ packages << "\\usepackage{" << feature << "}\n";
0/ in this particular case, it seems to me that the variable
nb_simplefeatures can be removed.
1/ I prefer an explict type name (char const * here) rather than auto,
excecpt of course if the type is an ugly template expression. I find it
more precise and easier to read).
2/ please constify those expression whenever possible.
There are also in the Qt part things like:
+ for (auto pcat : pcats) {
Is skipping the "const &" intended because of some Qt specifities?
On a different subject:
- QPushButton * okay_;
- QPushButton * apply_;
- QPushButton * cancel_;
- QPushButton * restore_;
- QCheckBox * auto_apply_;
- QPushButton * default_;
+ QPushButton * okay_{nullptr};
+ QPushButton * apply_{nullptr};
+ QPushButton * cancel_{nullptr};
+ QPushButton * restore_{nullptr};
+ QCheckBox * auto_apply_{nullptr};
+ QPushButton * default_{nullptr};
We use the form "= nullptr" in the source (for now). Any reason why the
weird {} notation is better?
JMarc
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel