Review Request 120254: [Kalzium] Minor fixes, reorganize includes, some cleanups

Martin Walch walch.martin at web.de
Fri Sep 19 00:36:58 UTC 2014 


> On Sept. 18, 2014, 9:16 nachm., Albert Astals Cid wrote:
> > compoundviewer/kalziumglwidget.cpp, line 17
> > <https://git.reviewboard.kde.org/r/120254/diff/1/?file=312951#file312951line17>
> >
> >     Don't do this, not sure where we recomment it, but it's really a bad idea, it will introduce problems with Qt5 porting where some classes have the same name but have been moved to a different module.

Canonical include names look like a good idea to me (e. g. krazy2 can easily detect all duplicate includes). As reference I took the style guide for kdelibs (which is also referenced in the kdepim style guide)
https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes

I will remove the leading Qt modules from the include names.


> On Sept. 18, 2014, 9:16 nachm., Albert Astals Cid wrote:
> > compoundviewer/kalziumglpart.h, line 18
> > <https://git.reviewboard.kde.org/r/120254/diff/1/?file=312949#file312949line18>
> >
> >     These changes look a bit arbitrary, why did you decide to change them?

Just as for the Qt imports, there can be found two forms of KDE includes: <kdeclass.h> and <KDEClass>. For canonicalization I changed them all to the same format. And I chose to use <KDEClass> instead of <kdeclass.h>, so it is the same style as for Qt includes.

Is this bad?


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120254/#review66877
-----------------------------------------------------------


On Sept. 18, 2014, 5:44 vorm., Martin Walch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120254/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2014, 5:44 vorm.)
> 
> 
> Review request for KDE Edu.
> 
> 
> Repository: kalzium
> 
> 
> Description
> -------
> 
> * src/calculator/titrationCalculator.cpp:
> 
> replace
> > if (!texto) {
> >    QMessageBox::critical(this, "Error", "Unable to open " + file);
> > }
> > if (texto) {
> 
> with
> > if (!texto) {
> >    QMessageBox::critical(this, "Error", "Unable to open " + file);
> > } else {
> 
> to emphasize that they are logically alternative
> 
> * src/calculator/titrationCalculator.cpp:
> Fix "warning: 'cn' may be used uninitialized in this function".
> As far as I see, this has no proper initialization at all and should
> horribly fail. However, in all my tests it behaved as if it was properly
> initialized with 0.0. Maybe there is some compiler (gcc) magic at work.
> Nevermind. Just fix it.
> 
> * further cleanup of includes:
> replace
> > #include <qclass.h>
> with
> > #include <QtModule/QClass>
> 
> replace
> > #include <QClass>
> with
> > #include <QtModule/QClass>
> 
> replace
> > #include <kclass.h>
> with
> > #include <KClass>
> 
> replace
> > #include <QtModule>
> with the classes of QtModule that are actually necessary.
> 
> use forward declarations and move includes into cpp files
> 
> remove duplicate or unused includes
> 
> (As a side note: single threaded build time reduced by roughly 5%)
> 
> * Drop C bindings in favor of explicit C++ bindings
> (e.g. include cmath, not math.h and use std namespace)
> 
> 
> Diffs
> -----
> 
>   src/unitsettingsdialog.cpp 4f50aca 
>   src/tableinfowidget.h dcfeea7 
>   src/tableinfowidget.cpp 48eb865 
>   src/tablesdialog.h a04f4ad 
>   src/tablesdialog.cpp 6dd126e 
>   src/tools/moleculeview.h 73bf9b8 
>   src/tools/moleculeview.cpp 7a6c9f3 
>   src/tools/moleculewidgetplugin.h c6f1289 
>   src/tools/obconverter.cpp 04b557d 
>   src/unitsettingsdialog.h 04d5cf0 
>   src/psetable/periodictableview.h 366480e 
>   src/psetable/periodictableview.cpp 9669e5b 
>   src/psetable/statemachine.h 0cad142 
>   src/psetable/statemachine.cpp 9ce8d58 
>   src/rsdialog.h 930276c 
>   src/rsdialog.cpp 64a6b60 
>   src/search.h 83bcf66 
>   src/search.cpp 7ff6464 
>   src/searchwidget.h 66613ae 
>   src/searchwidget.cpp 17edc1b 
>   src/spectrumviewimpl.cpp f9c0488 
>   src/spectrumwidget.h 4caecb4 
>   src/spectrumwidget.cpp 2264590 
>   src/psetable/periodictablestates.h f8258c3 
>   src/psetable/periodictablestates.cpp 501b75e 
>   src/orbitswidget.cpp d94ccf1 
>   src/psetable/elementitem.h 7dd7e30 
>   src/psetable/elementitem.cpp a2b238c 
>   src/psetable/numerationitem.h 5cf8afb 
>   src/psetable/numerationitem.cpp 574f74f 
>   src/psetable/periodictablescene.h c89044b 
>   src/psetable/periodictablescene.cpp 5ea99c0 
>   src/molcalcwidget.h a0581a0 
>   src/molcalcwidget.cpp 784928a 
>   src/orbitswidget.h 17fac9c 
>   compoundviewer/kalziumglpart.h 0881df1 
>   compoundviewer/kalziumglpart.cpp 001399d 
>   compoundviewer/kalziumglwidget.cpp 5e7f592 
>   compoundviewer/openbabel2wrapper.cpp 85406e1 
>   libscience/chemicaldataobject.h 55a40dc 
>   libscience/chemicaldataobject.cpp 8fac92c 
>   libscience/element.h 43979d9 
>   libscience/element.cpp 762462f 
>   libscience/elementparser.h f3a8130 
>   libscience/elementparser.cpp 7c5ecaf 
>   libscience/isotope.cpp 91db538 
>   libscience/isotopeparser.h c59aff4 
>   libscience/isotopeparser.cpp 0e646c1 
>   libscience/moleculeparser.h 3a41b8b 
>   libscience/moleculeparser.cpp 35321cc 
>   libscience/parser.h 9c9e209 
>   libscience/parser.cpp e34f189 
>   libscience/psetables.h 540e654 
>   libscience/psetables.cpp d7e12a0 
>   libscience/spectrum.h 3d43d6f 
>   libscience/spectrum.cpp 9ce97a5 
>   libscience/spectrumparser.h 1e60a1c 
>   libscience/spectrumparser.cpp c89c114 
>   libscience/tests/isotopereadingtest.cpp fbafdf4 
>   libscience/tests/spectrumreadingtests.cpp 5a5edd3 
>   plasmoid/applet/bodr/kalzium_plasma.h 94083e2 
>   plasmoid/applet/bodr/kalzium_plasma.cpp 6f312b3 
>   plasmoid/applet/concentrationPlasmoid/concentrationCalculator.h 5c27286 
>   plasmoid/applet/concentrationPlasmoid/concentrationCalculator.cpp ac7f55b 
>   plasmoid/applet/didyouknow/didyouknow.h 4f402fe 
>   plasmoid/applet/didyouknow/didyouknow.cpp 11a04a8 
>   plasmoid/applet/gasPlasmoid/gasCalculator.h f552742 
>   plasmoid/applet/gasPlasmoid/gasCalculator.cpp de78078 
>   plasmoid/applet/nuclearPlasmoid/kalziumdataobject.h 9189dea 
>   plasmoid/applet/nuclearPlasmoid/kalziumdataobject.cpp e802100 
>   plasmoid/applet/nuclearPlasmoid/nuclearCalculator.h d6ced65 
>   plasmoid/applet/nuclearPlasmoid/nuclearCalculator.cpp dfcbc6d 
>   plasmoid/applet/psePlasmoid/Molmasscalculator.h ccd80e5 
>   plasmoid/applet/psePlasmoid/Molmasscalculator.cpp 4b6acbe 
>   plasmoid/applet/psePlasmoid/Periodictable.h 5c130cc 
>   plasmoid/applet/psePlasmoid/Periodictable.cpp 5b75778 
>   plasmoid/engine/kalzium_engine.h 3e46481 
>   plasmoid/engine/kalzium_engine.cpp 36fce6f 
>   src/calculator/calculator.h cda195d 
>   src/calculator/calculator.cpp efe57aa 
>   src/calculator/concCalculator.h 54c1fa1 
>   src/calculator/concCalculator.cpp 785e77a 
>   src/calculator/gasCalculator.h 4cbcecf 
>   src/calculator/gasCalculator.cpp adef7f5 
>   src/calculator/nuclearCalculator.h f0230f8 
>   src/calculator/titrationCalculator.h 9880ebe 
>   src/calculator/titrationCalculator.cpp 6c67887 
>   src/detailedQmlView.h 6c30912 
>   src/detailedQmlView.cpp 50321f5 
>   src/detailedgraphicaloverview.h 49d9980 
>   src/detailedgraphicaloverview.cpp 5fb716b 
>   src/detailinfodlg.h e81802b 
>   src/detailinfodlg.cpp 5932a7f 
>   src/elementdataviewer.h dba156e 
>   src/elementdataviewer.cpp 49a2a18 
>   src/eqchemview.h 59d3384 
>   src/eqchemview.cpp ebce0be 
>   src/exportdialog.h 7aa5543 
>   src/exportdialog.cpp a21dc26 
>   src/gradientwidget_impl.h 0521765 
>   src/gradientwidget_impl.cpp 622717a 
>   src/isotopetable/informationitem.h d8eae50 
>   src/isotopetable/isotopeguideview.h 560f3ba 
>   src/isotopetable/isotopeguideview.cpp d96ce80 
>   src/isotopetable/isotopeitem.h 5b7840b 
>   src/isotopetable/isotopeitem.cpp b98155d 
>   src/isotopetable/isotopescene.h 3531efd 
>   src/isotopetable/isotopescene.cpp 18f6a54 
>   src/isotopetable/isotopetabledialog.h 4f075c5 
>   src/isotopetable/isotopetabledialog.cpp 83fb000 
>   src/isotopetable/isotopeview.h 54d3d97 
>   src/isotopetable/isotopeview.cpp dd6bc8e 
>   src/kalzium.h 86e4a6d 
>   src/kalzium.cpp 2e85cf8 
>   src/kalziumdataobject.h 0d88856 
>   src/kalziumdataobject.cpp e962670 
>   src/kalziumelementproperty.h f8b3a67 
>   src/kalziumelementproperty.cpp c42ecd5 
>   src/kalziumgradienttype.h 1089ed7 
>   src/kalziumgradienttype.cpp 5130f85 
>   src/kalziumnumerationtype.h 842f144 
>   src/kalziumnumerationtype.cpp 52275d0 
>   src/kalziumschemetype.h a6447d5 
>   src/kalziumschemetype.cpp d35a97c 
>   src/kalziumunitcombobox.h 717be9e 
>   src/kalziumutils.h 0938b34 
>   src/kalziumutils.cpp bae9c54 
>   src/kdeeduglossary.h 3b47aee 
>   src/kdeeduglossary.cpp f561568 
>   src/legendwidget.h 9794f07 
>   src/legendwidget.cpp b0a7ba0 
>   src/main.cpp ff492f0 
> 
> Diff: https://git.reviewboard.kde.org/r/120254/diff/
> 
> 
> Testing
> -------
> 
> * compiling
> * running program
> 
> 
> Thanks,
> 
> Martin Walch
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140919/27c100ea/attachment-0001.html>

More information about the kde-edu mailing list