Details
- Reviewers
asemke
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
src/backend/worksheet/plots/cartesian/XYCurve.cpp | ||
---|---|---|
1690 | should we define this static? | |
1744 | In indexForX() you have handling for datetime. Why not to add this here, too? | |
1766 | this three lines can be simplified to bool increase = (properties != AbstractColumn::Properties::MonotonicDecreasing); | |
1773 | maxSteps, camel case... | |
src/backend/worksheet/plots/cartesian/XYCurve.h | ||
72 | indexForX oder indexForXinColumn? Let's maybe name all these functions simply indexForX and diferentiation is done by the parameters and the documentation of these overloaded parameters. This would be similar for example to the all those map*() functions in CartesianCoordinateSystem. |
src/backend/worksheet/plots/cartesian/XYCurve.cpp | ||
---|---|---|
1744 | should there datetimes on the y axis? |
- renamed functions
- renamed max_steps to maxSteps
- made calculateMaxSteps to camel case
- simplified some code
Causes build failures on KDE Neon builds.
https://build.neon.kde.org/job/bionic_unstable_extras_labplot/188/
00:40:09 /workspace/build/src/backend/worksheet/plots/cartesian/XYCurve.cpp: In static member function ‘static int XYCurve::calculateMaxSteps(unsigned int)’:
00:40:09 /workspace/build/src/backend/worksheet/plots/cartesian/XYCurve.cpp:1708:2: error: narrowing conversion of ‘-1’ from ‘int’ to ‘char’ inside { } [-Wnarrowing]
00:40:09 };
00:40:09 ^
Does it solve to explicitly set: "const char LogTable256[256]" to "const signed char LogTable256[256]" ?
Yes. Whether char is signed or unsigned is implementation-dependent. Using "signed char" should fix this.
I included calculateMaxSteps() as nsl_sf_basic_log2p1_int() in NSL. Please use this function now.
I will add performance and accuracy tests for different implementations soon.