gooroodev commented on PR #19602:
URL: https://github.com/apache/echarts/pull/19602#issuecomment-2131318923
### 1. Summary of Changes
The pull request addresses an issue where stacked areas overflow on null
values when the `connectNulls` option is set to true. The changes involve
updating the `getStackedOnPoint` function in `src/chart/line/helper.ts` to
handle null values more gracefully by:
- Separating the retrieval of `stackedOverValue` and `stackResultValue`.
- Adding a condition to check for `stackedOverValue` and `stackResultValue`
to determine if `stackedOverValue` should be set to `dataCoordInfo.valueStart`.
### 2. Issues, Bugs, or Typos
#### Issue 1: Unnecessary Check on `dataCoordInfo.stacked`
The condition `dataCoordInfo.stacked && isNaN(stackResultValue)` is
redundant since `dataCoordInfo.stacked` is already checked before retrieving
`stackResultValue`.
**Proposed Improvement:**
Remove the redundant `dataCoordInfo.stacked` check in the condition.
```diff
- if (isNaN(stackedOverValue) && !(dataCoordInfo.stacked &&
isNaN(stackResultValue))) {
+ if (isNaN(stackedOverValue) && !isNaN(stackResultValue)) {
```
### 3. General Review of Code Quality and Style
#### Positive Aspects:
- The code changes are clear and well-targeted at solving the specific issue.
- Variable names are meaningful and improve readability.
#### Suggestions for Improvement:
- Consistency in formatting: Ensure consistent use of indentation and
spacing.
- Comments: Adding comments to explain the logic, especially the condition
handling, would improve maintainability.
**Example of Improved Code with Comments:**
```typescript
export function getStackedOnPoint(
dataCoordInfo: DataCoordInfo,
coordSys: CoordinateSystem,
data: SeriesData,
idx: number
) {
let stackedOverValue = NaN;
let stackResultValue = NaN;
// Retrieve stacked values if stacking is enabled
if (dataCoordInfo.stacked) {
stackedOverValue = data.get(
data.getCalculationInfo('stackedOverDimension'),
idx
) as number;
stackResultValue = data.get(
data.getCalculationInfo('stackResultDimension'),
idx
) as number;
}
// If stackedOverValue is NaN but stackResultValue is valid, use
valueStart
if (isNaN(stackedOverValue) && !isNaN(stackResultValue)) {
stackedOverValue = dataCoordInfo.valueStart;
}
const baseDataOffset = dataCoordInfo.baseDataOffset;
const stackedData = [];
stackedData[baseDataOffset] = data.get(dataCoordInfo.baseDim, idx);
stackedData[1 - baseDataOffset] = stackedOverValue;
return coordSys.dataToPoint(stackedData);
}
```
Overall, the pull request effectively addresses the issue, but minor
improvements in condition handling and code comments would enhance its quality.
Yours, [Gooroo.dev](https://gooroo.dev). To receive reviews automatically,
[install Github App](https://github.com/apps/gooroo-dev)
--
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]
