Skip to content

ref: simplify node occurrence check in validationImpossibleOneway#11943

Open
dipanshurdev wants to merge 2 commits intoopenstreetmap:developfrom
dipanshurdev:dev02
Open

ref: simplify node occurrence check in validationImpossibleOneway#11943
dipanshurdev wants to merge 2 commits intoopenstreetmap:developfrom
dipanshurdev:dev02

Conversation

@dipanshurdev
Copy link

Description

Optimize node occurrence detection in impossible oneway validation

Improves code efficiency and readability by replacing a manual loop
with native Array methods. The function now uses indexOf/lastIndexOf
comparison to detect duplicate nodes in a way.

Type of Change

  • Performance optimization
  • Code quality improvement
  • Bug fix
  • New feature

Changes

  • Replaced manual loop iteration with indexOf() and lastIndexOf() comparison
  • Reduced from 7 lines to 1 line
  • More idiomatic JavaScript using native array methods
  • No behavioral changes - same validation logic

Performance Impact

  • Single algorithm pass instead of iterating array
  • Leverages optimized native browser/runtime methods
  • Reduces memory allocation (no counter variable)

Testing

  • No functional changes - same validation behavior
  • Detects duplicate nodes the same way
  • Code is clearer and more maintainable

}
}
return false;
return way.nodes.indexOf(nodeID) !== way.nodes.lastIndexOf(nodeID);
Copy link
Member

Choose a reason for hiding this comment

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

This would not return true when the nodeID is not in the way nodes list. So, at the very least it should be something like this

            const firstIndex = way.nodes.indexOf(nodeID);
            return firstIndex !== -1 && firstIndex !== way.nodes.lastIndexOf(nodeID);

That said, it would be interesting to know if the new approach is any faster than the old for loop. Can you do some benchmarking and check, please?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants