<table><tr><td style="">ervin requested changes to this revision.<br />ervin added inline comments.<br />This revision now requires changes to proceed.
</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/D26046">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/D26046#inline-151426">View Inline</a><span style="color: #4b4d51; font-weight: bold;">managedconfigmodule.cpp:175</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">it</span><span style="color: #aa2211">-></span><span class="n">isNull</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span class="n">d</span><span style="color: #aa2211">-></span><span class="n">_skeletons</span><span class="p">.</span><span class="n">erase</span><span class="p">(</span><span class="n">it</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">        <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #aa4000">void</span> <span class="n">ManagedConfigModule</span><span style="color: #aa2211">::</span><span class="n">settingsChanged</span><span class="p">()</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    <span style="color: #aa4000">bool</span> <span class="n">needsSave</span> <span style="color: #aa2211">=</span> <span style="color: #304a96">false</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    <span style="color: #aa4000">bool</span> <span class="n">representsDefaults</span> <span style="color: #aa2211">=</span> <span style="color: #304a96">true</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    <span style="color: #aa4000">for</span> <span class="p">(</span><span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span> <span style="color: #a0a000">skeleton</span> <span class="p">:</span> <span class="n">qAsConst</span><span class="p">(</span><span class="n">d</span><span style="color: #aa2211">-></span><span class="n">_skeletons</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">        <span class="n">needsSave</span> <span style="color: #aa2211">|=</span> <span class="n">skeleton</span><span style="color: #aa2211">-></span><span class="n">isSaveNeeded</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">        <span class="n">representsDefaults</span> <span style="color: #aa2211">&=</span> <span class="n">skeleton</span><span style="color: #aa2211">-></span><span class="n">isDefaults</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">needsSave</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">        <span class="n">needsSave</span> <span style="color: #aa2211">=</span> <span class="n">isSaveNeeded</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Careful there you should use the return value of erase(). It invalidates iterators so it's probably working just by change here (I'd suspect it might crash for instance if it happens to remove the last one in the list).</p>

<p style="padding: 0; margin: 8px;">Even better yet you could spare that loop completely and use remove_if + erase, see:<br />
<a href="https://en.wikipedia.org/wiki/Erase-remove_idiom" class="remarkup-link" target="_blank" rel="noreferrer">https://en.wikipedia.org/wiki/Erase-remove_idiom</a></p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R296 KDeclarative</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D26046">https://phabricator.kde.org/D26046</a></div></div><br /><div><strong>To: </strong>crossi, Plasma, Frameworks, ervin, bport, davidedmundson, mart<br /><strong>Cc: </strong>meven, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>