D18420: Optimize and reduce code in GuiObserver

Alexander Semke noreply at phabricator.kde.org
Tue Jan 22 21:16:59 GMT 2019


asemke added a comment.


  In D18420#397243 <https://phabricator.kde.org/D18420#397243>, @croick wrote:
  
  > Would you agree on such a change?
  
  
  Thanks for addressing this. This part grew historically with the help of copy&paste. It's time to do some cleanup here :-)
  
  > `case "Worksheet"_hash:` is still human-readable (compare to `if (className == "Worksheet")`.
  >  The very unlikely case of a hash collision would appear at compile time (if new dock widgets are introduced).
  
  It's human readable but it's even more readable if we use somethink like this:
  
    enum className {Spreadsheet, Worksheet, ...};
    map classNames{ {"Spreadsheet", Spreadsheet}, ...};
    
    switch (name)
         case Spreadsheet:
    ...
  
  Here we'd would need to register every class name in the enum and to do the mapping of the string class name to the enum value. This would result in some additional maintenance (you need to know that you need to register the new class here) but at costs of better readability. And maybe we can use this (global) enum in future at other places where we probably do some dirty workarounds now because we don't have such an enum.
  
  > On the other hand the selection of the right dock is now more efficient than before.
  
  I don't think the performance is relevant here. I'd rather go for a greater readability of the code. Performance-wise I'd say a simple lookup in the map as mentioned above is faster than the calculation of the hash.
  
  Would you agree with such a solution?

REPOSITORY
  R262 LabPlot

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

To: croick, #labplot
Cc: asemke, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190122/171d20af/attachment.html>


More information about the kde-edu mailing list