Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split PathHierarchy implementation into different files. #662

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

d-ronnqvist
Copy link
Contributor

@d-ronnqvist d-ronnqvist commented Jul 17, 2023

Bug/issue #, if applicable:

Summary

This splits the PathHierarchy implementation into separate files for various responsibilities.

This was previously all in one file to keep as much as possible of its implementation file private. The current length (~1500 lines) is starting to approach the point where the file length makes the code unnecessarily hard to follow. I would have been fine as-is, but some new features and enhancements that I've been working, when added together, start to make this a 2500-3000 line file, which has a real impact on the readability of it.

Splitting this into smaller files meant exposing a little bit of the API, but I find that it still sufficiently encapsulated and that the code readability improvements of spitting into smaller files are worth it.


Ignoring the moved—but unchanged—code, these are the actual changes in this PR:

  • The access level of the top-level DisambiguationTree struct is changed from fileprivate to internal so that the other files can use it. It is also:

    • nested within PathHierarchy to indicate that its functionality is specific to that.
    • renamed to DisambiguationContainer to better reflect that its implementation may not always represent a tree.
  • The read access level of thechildren property on PathHierarchy.Node is changed from fileprivate to internal. The write access level remains private(set).

  • The availableChildren data for PathHierarchy.Error.unknownName is changed from an array to a set. This makes it consistent with PathHierarchy.Error.notFound and better reflect that the order doesn't matter because the values are sorted by near-miss proximity to the next path component in the authored link.

Dependencies

None.

Testing

None. This isn't a user-facing change.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • [ ] Added tests
  • Ran the ./bin/test script and it succeeded
  • [ ] Updated documentation if necessary

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit d756924 into swiftlang:main Aug 21, 2023
@d-ronnqvist d-ronnqvist deleted the divide-path-hierarchy-file branch August 21, 2023 20:39
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