Skip to content

Conversation

@eckter
Copy link
Contributor

@eckter eckter commented Nov 21, 2025

I chose earlier to not include the object length to the ranges, for "memory concerns". But I now regret that choice, especially because directed/undirected projections would be a lot easier with object lengths.

And we had redundant variables anyway, so it could fit without adding an extra one.

@eckter eckter requested a review from Erashin November 21, 2025 09:17
@eckter eckter requested a review from a team as a code owner November 21, 2025 09:17
@github-actions github-actions bot added the area:core Work on Core Service label Nov 21, 2025
Copy link
Member

@hhirtz hhirtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build and tests fine locally

init {
require(length >= 0.meters)
require(pathEnd - pathBegin == length)
require(objectEnd <= objectLength)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the range always within the object? Why can objectBegin be negative but objectEnd cannot go past the object's length? just asking, since it doesn't seem we build a range with objectBegin < 0 in tests

Comment on lines +50 to +54
val pathEnd: Offset<TrainPath>
get() = pathBegin + length

val objectEnd: Offset<OffsetType>
get() = objectBegin + length
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing doc comments that have been removed from the default constructor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core Work on Core Service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants