Fix core-geometry lint issues #8521
Conversation
saeeedtorabi
requested review from
a team,
aruniverse and
ben-polinsky
as code owners
| * Return 0 if inputs are invalid or calculation fails. | ||
| */ | ||
| public override curveLengthBetweenFractions(fraction0: number, fraction1: number): number { | ||
| // TODO: lint fix changes in this function must be verified |
There was a problem hiding this comment.
I'm just putting a comment here to make sure someone takes action on this.
| * * For type checking, assume array is not empty. | ||
| */ | ||
| public front(result?: Point3d): Point3d { return this.xyz.front(result)!; } | ||
| public front(result?: Point3d): Point3d { return this.xyz.frontUnchecked(result); } |
There was a problem hiding this comment.
As far as I can tell, this will now return a zero Point3d if this.xyz is empty, while before it would have returned undefined. Obviously the old behavior was wrong, but should it throw an exception instead of returning a zero point? (Note: I don't know the correct answer to this question; I'm just verifying that the new behavior is the desired behavior.) If you want it to throw an exception, have it do this:
return expectDefined(this.xyz.front(result));If you update this function, you should update back in the same way to be consistent.
There was a problem hiding this comment.
@dassaf4 what do you think?
| private constructor(graph: HalfEdgeGraph, tolerance: number) { | ||
| this._graph = graph; | ||
| this._edgeSet = MarkedEdgeSet.create(graph)!; | ||
| this._edgeSet = MarkedEdgeSet.create(graph) as MarkedEdgeSet; |
There was a problem hiding this comment.
Another inappropriate as.
There was a problem hiding this comment.
@dassaf4 If graph.grabMask() returns HalfEdgeMask.NULL_MASK then MarkedEdgeSet.create(graph) would return undefined so how can we guarantee that this does not return undefined?
There was a problem hiding this comment.
Since it hasn't produced problems up until now, I would suggest just wrapping it in expectDefined. If it is undefined, expectDefined will throw an exception.
core/geometry/src/curve/internalContexts/CurveCurveIntersectXY.ts
Outdated
Show resolved
Hide resolved
3 participants
No description provided.