D22507: Tooltip Framework for m_statsTable in HypothesisTestView

Alexander Semke noreply at phabricator.kde.org
Wed Jul 17 21:21:48 BST 2019


asemke added inline comments.

INLINE COMMENTS

> HypothesisTest.cpp:1578
> +        if (!currCell->tooltip.isEmpty())
> +            tooltips->insert(currCell->data, currCell->tooltip);
>      }

is currCell->data unique or can we have collisions here?

> HypothesisTest.h:85
>  	QString statsTable();
> +    QMap<QString, QString>* tooltips();
>  

why do you want to work with a pointer here? You create the map with new in the constructor and never delete it. Just create the map on the stack, which is ok for this use-case, and you don't need to bother with the memory management.

> HypothesisTestPrivate.h:59
>          int columnSpanCount;
> -        bool isHeader;
> -        Cell(QVariant data = "", int level = 0, bool isHeader = false, int rowSpanCount = 1, int columnSpanCount = 1) {
> +        Cell(QVariant data = "", int level = 0, bool isHeader = false, QString tooltip = "", int rowSpanCount = 1, int columnSpanCount = 1) {
>              this->data = data.toString();

you have quite a lot of parameters already in the constructor. Consider setting the tooltip via setToolTip() function. No need to add yet another empty parameter to the constructor.

REPOSITORY
  R262 LabPlot

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

To: devanshuagarwal, sgerlach, asemke
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190717/f24d103b/attachment-0001.html>


More information about the kde-edu mailing list