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

Reply via email to