D13665: Histogram bug fixes

Alexander Semke noreply at phabricator.kde.org
Sat Jun 23 05:44:49 UTC 2018


asemke added inline comments.

INLINE COMMENTS

> Histogram.cpp:155
>  
> -void Histogram::setHistrogramType(Histogram::HistogramType histogramType) {
> +void Histogram::setHistogramType(Histogram::HistogramType histogramType) {
>  	d_ptr->histogramType = histogramType;

Use a std-setter class here defined via out macros to the the undo/redo-stuff.

> Histogram.cpp:157
>  	d_ptr->histogramType = histogramType;
> +	emit HistogramDataChanged();
>  	DEBUG(histogramType);

Once you switched to the std-setter, retransform() needs to be called, no need to emit this signal anymore.

> Histogram.cpp:161
>  
> +void Histogram::setBarsType(Histogram::BarsType barsType) {
> +    Q_D(Histogram);

similar here, use std-setter.

> Histogram.cpp:172
>  
> +Histogram::BarsType Histogram::getBarsType() {
> +	return d_ptr->m_barsType;

for the getters we also use macros. Check how all other getters are declared and defined.

> Histogram.cpp:176
> +
>  void Histogram::setbinsOption(Histogram::BinsOption binsOption) {
>  	d_ptr->histogramData.binsOption = binsOption;

also here, use std-setter with retransform(), no need for this emit.

> Histogram.cpp:180
>  }
>  void Histogram::setBinValue(int binValue) {
>  	d_ptr->histogramData.binValue= binValue;

also here, use std-setter with retransform(), no need for this emit.

> Histogram.h:48
>  	enum HistogramType {Ordinary,Cumulative, AvgShift};
> +	enum BarsType {Vertical, Horizontal};
>  

HistorgramOrientation would be a better name or this enum, similar to AxisOrientation.

REPOSITORY
  R262 LabPlot

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

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


More information about the kde-edu mailing list