100pah commented on code in PR #20184:
URL: https://github.com/apache/echarts/pull/20184#discussion_r1828097788
##########
src/coord/polar/polarCreator.ts:
##########
@@ -102,9 +102,15 @@ function updatePolarScale(this: Polar, ecModel:
GlobalModel, api: ExtensionAPI)
// Fix extent of category angle axis
if (angleAxis.type === 'category' && !angleAxis.onBand) {
const extent = angleAxis.getExtent();
- const diff = 360 / (angleAxis.scale as OrdinalScale).count();
- angleAxis.inverse ? (extent[1] += diff) : (extent[1] -= diff);
- angleAxis.setExtent(extent[0], extent[1]);
+ const angleModel = angleAxis.model;
+ const endAngle = angleModel.get('endAngle');
+ const spanAngle = (endAngle == null ? 360 : endAngle -
angleModel.get('startAngle'))
Review Comment:
The conditional operator has lower precedence than arithmetic operator.
`endAngle == null ? 360 : endAngle - angleModel.get('startAngle')`
means
`endAngle == null ? 360 : (endAngle - angleModel.get('startAngle'))`
This should be a "famous" and commonly occurring mistake in not only JS but
also Java, c++, ...
And why there is null checker for only `endAngle` but not `startAngle`?
I think in this place we can assume that both `endAngle` and `startAngle`
are not nullable here. I mean that set default value in only one place
(function `setAxis`).
And should we retrieve value from `extent` rather from angleModel here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
