D17403: Copy root value or (x,y) pair to clipboard

Albert Astals Cid noreply at phabricator.kde.org
Sun Dec 9 21:19:49 GMT 2018


aacid added inline comments.

INLINE COMMENTS

> maindlg.h:219
> +	/// Root value for copying into clipboard
> +	double m_rootValue;
> +

for consistence i'd initialzie  this in the constructor to 0 or something

> view.cpp:3519
>  			else
> +            {
>  				m_haveRoot=false;

spacing here looks weird to me and looks like it should be on the same horizontal position as "else" but since you didn't use arc i can't see the rest of the file to see if that's actually the style for the file, feel free to ignore this.

> view.h:215
> +		 */
> +		QPointF m_crosshairPosition;
> +

please add a getter, don't make this member variable public.

> view.h:243
>  		void setStatusBarText(const QString &);
> +		void setRootValue(bool haveRoot, double rootValue);
>  	

i was going to complain how callign a signal setXXX makes no sense, because the signal is not setting anything, it's just saying something happened, so it should be really called rootValueChanged or updateRootValue or something like that but i see the one above is call set, so meh, i guess leave it if you don't agree with rootValueChanged being a better name.

REPOSITORY
  R334 KmPlot

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

To: yurchor, #kde_edu
Cc: aacid, kde-doc-english, kde-edu, skadinna, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20181209/387e611f/attachment.html>


More information about the kde-edu mailing list