<table><tr><td style="">kfunk accepted this revision.<br />kfunk added a comment.<br />This revision is now accepted and ready to land.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D2075" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D2075#76591" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D2075#76591</a>, <a href="https://phabricator.kde.org/p/jonathans/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@jonathans</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>They interrupt the logical flow of the code, and because the conditionally executed code no longer sits inside an indented block, make it less evident to the reader that it is conditionally executed.</p></div>
</blockquote>
<p>Interesting claim that "not using early returns" makes the code more readable -- please have a look at the discussion on <a href="http://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement" class="remarkup-link" target="_blank" rel="noreferrer">http://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement</a> -- I agree that you could consider KFileDialog having modes of operations though -- there having early returns could limit the extensibility in case you ever wanted to introduce another mode.</p>
<p>Anyhow, I don't care too much for this deprecated code base. This patch looks fine semantically, and fixes a bug => so let's go for it.</p>
<p>I'd wait for another +1 from someone else.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R239 KDELibs4Support</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2075" rel="noreferrer">https://phabricator.kde.org/D2075</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>jonathans, Frameworks, dfaure, kfunk<br /><strong>Cc: </strong>kfunk, aacid<br /></div>