D11992: Fix bad performance for `addRules` in `DefaultHighlighter` for big containers

Alexander Semke noreply at phabricator.kde.org
Sat Apr 7 10:00:14 UTC 2018


asemke added a comment.


  In D11992#241782 <https://phabricator.kde.org/D11992#241782>, @sirgienko wrote:
  
  > In D11992#241763 <https://phabricator.kde.org/D11992#241763>, @asemke wrote:
  >
  > > Can we try to implement this with a boolean paramer m_suppressRuleChangedSignal instead of adding new functions? In addRules() we set m_suppressRuleChangedSignal = true and use in addRule()
  > >
  > >   if (!m_supressRuleChangedSignal)
  > >        emit rulesChanged();
  > >
  > >
  > > The code will be more compact and clean with this. Also, this would be similar to the convention we use in LabPlot <https://phabricator.kde.org/project/profile/56/>.
  >
  >
  > Done
  
  
  This is not exactly what I meant. bool m_suppressRuleChangeSignal is a private class member (that's why m_ in the name) which is initialized in the constructor to false. We don't need to change the function signatures. We only add
  
    m_suppressRuleChangedSignal = true;
    for (;i != end; ++i)
        addRule(*i, format);
    m_suppressRuleChangedSignal = false;
  
  in DefaultHighlither::addRules() and that
  
    if (!m_suppressRuleChangedSignal)
        emit ruleChanged();
  
  that you already have now in DefaultHighlighter::addRule(). And similar for removeRule-stuff. No need to change the signatures of the already available functions.
  
  Since we don't have any private members in DefaultHighlighter except of the add pointers, simply add bool suppressRuleChangedSignal to DefaultHighlighterPrivate instead of adding m_suppressRuleChangedSignal to DefaultHighlighter and use it via d->suppressRuleChangedSignal.

REPOSITORY
  R55 Cantor

REVISION DETAIL
  https://phabricator.kde.org/D11992

To: sirgienko, #cantor, asemke
Cc: #cantor, #kde_edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180407/94e13b65/attachment.html>


More information about the kde-edu mailing list