<table><tr><td style="">meven added inline comments.
</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/D26398">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D26398#inline-149370">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ervin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">BlacklistedApplicationsModel.cpp:179</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I personally like that construct, but AFAIK it's rather unusual in KDE code, so maybe for the sake of the future developer use something more "classic".<br />
Either:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">const auto name = d->applications[i].name;
if (d->applications[i].blocked) {
    blockedApplications << name;
} else {
    allowedApplications << name;
}</pre></div>

<p style="padding: 0; margin: 8px;">Or:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">auto &list = (d->applications[i].blocked ? blockedApplications : allowedApplications);
list << d->applications[i].name;</pre></div>

<p style="padding: 0; margin: 8px;">Second option is closer to your initial intent (I think it's still less common than the first, but at least we guide the future reader understanding with the intermediate variable).</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It was just some copy/paste will update to the second option i tink</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D26398#inline-149378">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ervin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">BlacklistedApplicationsModel.cpp:186</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">It shall work as intended for now, but I think there's a potential caveat there in case of future refactoring and if the timing between save()/load() of the various settings object change. The isSaveNeeded()/isDefaults() state will be based on the whole settings object state. So we might claim here for saving being needed (currently unlikely AFAICT) or settings not being defaults (currently likely if load() brought values from the file which weren't defaults) based on config keys which are not managed by this class. I thus wonder if it wouldn't be wise to restrict that to only the items concerned.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I tride this, but I can't access here to the default value of  blockedApplications and allowedApplications as the generated function <tt style="background: #ebebeb; font-size: 13px;">defaultAllowedApplicationsValue_helper()</tt> is protected.<br />
I was missing an alternative.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R119 Plasma Desktop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D26398">https://phabricator.kde.org/D26398</a></div></div><br /><div><strong>To: </strong>meven, Plasma, ervin, bport<br /><strong>Cc: </strong>plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart<br /></div>