MantisBT - Doomseeker
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0003815Doomseeker[All Projects] Cleanuppublic2020-06-07 06:002020-06-08 13:42
ReporterWubTheCaptain 
Assigned To 
PrioritynoneSeveritytrivialReproducibilityhave not tried
StatusacknowledgedResolutionopen 
PlatformOSOS Version
Product Version1.3.1 
Target VersionFixed in Version 
Summary0003815: Found else statements after interrupted control flow
DescriptionThis is just a code style thing, so sort of trivial or a matter of undetermined preference.
Quote from clang-tidy
Please do not use else or else if after something that interrupts control flow - like return, break, continue, throw.

Refers to LLVM Coding Standards.
I personally agree with the clang-tidy suggestion here.
Steps To ReproduceSome hand-picked examples (not exhaustive):
--
src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp-            return gameParametersList();

src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp-    }
src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp:    else
--
src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp-            return NoneCanVote;

src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp-    }
src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp:    else if (rbSpectatorsCantVote->isChecked())

--
src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp-            return SpectatorsCantVote;

src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp-    }
src/plugins/zandronum/createserverdialogpages/votingsetupwidget.cpp:    else
--
src/plugins/zandronum/zandronumbinaries.cpp-            return ExeFile::pathToExe(message);
src/plugins/zandronum/zandronumbinaries.cpp-    }
src/plugins/zandronum/zandronumbinaries.cpp:    else
--

grep -B 2 -r "else" src/ | less can be somewhat helpful.
Additional Information'https://clang.llvm.org/extra/clang-tidy/checks/readability-delete-null-pointer.html [^]'

NB: That page's advice also contradict's clang-tidy's readability-braces-around-statements check (which is also a check I don't agree with). 🙂
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2020-06-07 06:00WubTheCaptainNew Issue
2020-06-07 20:45Pol MNote Added: 0021389
2020-06-07 20:45Pol MStatusnew => feedback
2020-06-08 01:21Blzut3Note Added: 0021406
2020-06-08 02:17WubTheCaptainStatusfeedback => acknowledged
2020-06-08 13:42ZalewaNote Added: 0021435

Notes
(0021389)
Pol M   
2020-06-07 20:45   
I personally see no special need to do this change, one could argue that it provides intent to add the else's, so I would not bother with this one (plus it will probably change the entirety of the files XD)
(0021406)
Blzut3   
2020-06-08 01:21   
I'm indifferent about this one. I personally do both styles based on what appears to be more readable for the particular block of code. Generally go with the recommendation here, but sometimes that needless else just looks right.
(0021435)
Zalewa   
2020-06-08 13:42   
Indifferent.