<table><tr><td style="">asemke 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/D18294">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/D18294#inline-101211">View Inline</a><span style="color: #4b4d51; font-weight: bold;">XYCurve.cpp:1690</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">int</span> <span class="n">XYCurve</span><span style="color: #aa2211">::</span><span class="n">calculateMaxSteps</span> <span class="p">(</span><span style="color: #aa4000">unsigned</span> <span style="color: #aa4000">int</span> <span class="n">value</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 style="color: #aa4000">const</span> <span style="color: #aa4000">char</span> <span class="n">LogTable256</span><span class="p">[</span><span style="color: #601200">256</span><span class="p">]</span> <span style="color: #aa2211">=</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">should we define this static?</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/D18294#inline-101209">View Inline</a><span style="color: #4b4d51; font-weight: bold;">XYCurve.cpp:1744</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">return</span> <span class="n">yColumn</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">valueAt</span><span class="p">(</span><span class="n">index</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span class="p">}</span> <span style="color: #aa4000">else</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">valueFound</span> <span style="color: #aa2211">=</span> <span style="color: #304a96">false</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">In indexForX() you have handling for datetime. Why not to add this here, too?</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/D18294#inline-101213">View Inline</a><span style="color: #4b4d51; font-weight: bold;">XYCurve.cpp:1766</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: #74777d">// bisects the index every time, so it is possible to find the value in log_2(rowCount) steps</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">bool</span> <span class="n">increase</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(151, 234, 151, .6);">    <span style="color: #aa4000">if</span><span class="p">(</span><span class="n">properties</span> <span style="color: #aa2211">==</span> <span class="n">AbstractColumn</span><span style="color: #aa2211">::</span><span class="n">Properties</span><span style="color: #aa2211">::</span><span class="n">MonotonicDecreasing</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this three lines can be simplified to</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);">bool increase = (properties != AbstractColumn::Properties::MonotonicDecreasing);</pre></div></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/D18294#inline-101212">View Inline</a><span style="color: #4b4d51; font-weight: bold;">XYCurve.cpp:1773</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">unsigned</span> <span style="color: #aa4000">int</span> <span class="n">max_steps</span> <span style="color: #aa2211">=</span> <span class="n">calculateMaxSteps</span><span class="p">(</span><span style="color: #aa4000">static_cast</span><span style="color: #aa2211"><</span><span style="color: #aa4000">unsigned</span> <span style="color: #aa4000">int</span><span style="color: #aa2211">></span><span class="p">(</span><span class="n">rowCount</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">maxSteps, camel case...</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/D18294#inline-101216">View Inline</a><span style="color: #4b4d51; font-weight: bold;">XYCurve.h:72</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">int</span> <span style="color: #004012">indexForX</span><span class="p">(</span><span style="color: #aa4000">double</span> <span class="n">x</span><span class="p">)</span> <span style="color: #aa4000">const</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span style="color: #aa4000">int</span> <span style="color: #004012">indexForX</span><span class="p">(</span><span style="color: #aa4000">double</span> <span class="n">x</span><span class="p">,</span> <span class="n">QVector</span><span style="color: #aa2211"><</span><span style="color: #aa4000">double</span><span style="color: #aa2211">>&</span> <span class="n">column</span><span class="p">,</span> <span class="n">AbstractColumn</span><span style="color: #aa2211">::</span><span class="n">Properties</span> <span class="n">properties</span> <span style="color: #aa2211">=</span> <span class="n">AbstractColumn</span><span style="color: #aa2211">::</span><span class="n">Properties</span><span style="color: #aa2211">::</span><span class="n">No</span><span class="p">)</span> <span style="color: #aa4000">const</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span style="color: #aa4000">int</span> <span style="color: #004012">indexForXinPointsVector</span><span class="p">(</span><span style="color: #aa4000">double</span> <span class="n">x</span><span class="p">,</span> <span class="n">QVector</span><span style="color: #aa2211"><</span><span class="n">QPointF</span><span style="color: #aa2211">>&</span> <span class="n">column</span><span class="p">,</span> <span class="n">AbstractColumn</span><span style="color: #aa2211">::</span><span class="n">Properties</span> <span class="n">properties</span> <span style="color: #aa2211">=</span> <span class="n">AbstractColumn</span><span style="color: #aa2211">::</span><span class="n">Properties</span><span style="color: #aa2211">::</span><span class="n">No</span><span class="p">)</span> <span style="color: #aa4000">const</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">indexForX oder indexForXinColumn? Let's maybe name all these functions simply indexForX and diferentiation is done by the parameters and the documentation of these overloaded parameters. This would be similar for example to the all those map*() functions in CartesianCoordinateSystem.</p></div></div></div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18294">https://phabricator.kde.org/D18294</a></div></div><br /><div><strong>To: </strong>Murmele, asemke<br /><strong>Cc: </strong>yurchor, kde-edu, Murmele, narvaez, apol<br /></div>