Review Request: KStars: fixed code checker issues #2
Akarsh Simha
akarsh.simha at kdemail.net
Sat Jan 5 08:40:56 UTC 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108192/#review24726
-----------------------------------------------------------
Okay, two things.
1. Most of the places where you "do nothing" in the if statement, you can remove the if statement altogether.
eg: if( fooDlg->exec() == QDialog::Accepted ) { // do nothing } can be replaced by fooDlg->exec() -- makes it more understandable to people, because otherwise one might wonder why that if is there.
2. If I understand the article you linked (http://blogs.kde.org/node/3919) correctly, it would be useful to put a:
if( !fooDlg ) return;
before every delete fooDlg;
After that, I think this is shippable.
kstars/dialogs/detaildialog.cpp
<http://git.reviewboard.kde.org/r/108192/#comment18980>
It might be wise to add a if( !dlg ) return; before each of the delete dlg statements.
kstars/kstarsactions.cpp
<http://git.reviewboard.kde.org/r/108192/#comment18979>
Why is this required?
kstars/printing/pwizfovbrowse.cpp
<http://git.reviewboard.kde.org/r/108192/#comment18981>
Again, the if(...) is not really needed.
I think no if( !dlg ) might be needed either.
kstars/printing/pwizfovsh.cpp
<http://git.reviewboard.kde.org/r/108192/#comment18982>
if(...) not needed.
Just have detailDlg->exec()
kstars/printing/pwizobjectselection.cpp
<http://git.reviewboard.kde.org/r/108192/#comment18983>
if( ... ) not needed. Just have
detailDlg->exec()
kstars/printing/pwizprint.cpp
<http://git.reviewboard.kde.org/r/108192/#comment18984>
Same
- Akarsh Simha
On Jan. 5, 2013, 7:48 a.m., Mohammed Nafees wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108192/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2013, 7:48 a.m.)
>
>
> Review request for KDE Edu and Kevin Krammer.
>
>
> Description
> -------
>
> http://www.google-melange.com/gci/task/view/google/gci2012/8192207
>
>
> Diffs
> -----
>
> kstars/dialogs/detaildialog.cpp efda624
> kstars/ekos/guide/gmath.cpp bf629a2
> kstars/ekos/guide/guider.cpp 30c6bcd
> kstars/ekos/guide/rcalibration.cpp 3340ae4
> kstars/ekos/guide/scroll_graph.cpp a37e81c
> kstars/kstarsactions.cpp 185507a
> kstars/printing/pwizfovbrowse.cpp 94edd09
> kstars/printing/pwizfovsh.cpp b5a4101
> kstars/printing/pwizobjectselection.cpp 50b9f5a
> kstars/printing/pwizprint.cpp 5c95318
> kstars/skyobjects/satellite.cpp 0ca44f7
> kstars/tools/conjunctions.cpp 6b8c081
> kstars/tools/modcalcvlsr.cpp 2a9b355
>
> Diff: http://git.reviewboard.kde.org/r/108192/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Mohammed Nafees
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20130105/2a6e87de/attachment-0001.html>
More information about the kde-edu
mailing list