<table><tr><td style="">asemke added a comment.
</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/D11962">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>I'd rather call appendEntry(CommandEntry::Type), etc. directly in Worksheet::load(QIODevice*) instead of going via all those append*Entry() functions - this additional indirection (one more function call)</p></blockquote>

<p>I am not sure, that compiler don't optimize it in inline call.</p></blockquote>

<p>The compiler will most probably inline those calls, yes.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;">

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>is of no benefit here and blocks my proposal for a simple fix for the problem you're addressing here.</p></blockquote>

<p>I think your solution simplier, but hachier too. I mean, in <tt style="background: #ebebeb; font-size: 13px;">appendCommandEntry</tt> logic of ignoring is obvicious, but from interface point of view, we sometimes ignore adding command (and only commands) entries (but don't ignore inserting or inserting before). And it's no good, I think, if caller don't know, what entry don't added, because we return <tt style="background: #ebebeb; font-size: 13px;">nullptr</tt> instead valid <tt style="background: #ebebeb; font-size: 13px;">WorksheetEntry*</tt>, and it cause probleams with dereference of nullptr pointer.</p></blockquote>

<p>This is a very valid argument.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R55 Cantor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11962">https://phabricator.kde.org/D11962</a></div></div><br /><div><strong>To: </strong>sirgienko, Cantor, asemke<br /><strong>Cc: </strong>Cantor, KDE Edu, narvaez, apol<br /></div>