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

Change subject / string type arguments #561

Open
joepio opened this issue Jan 20, 2023 · 0 comments
Open

Change subject / string type arguments #561

joepio opened this issue Jan 20, 2023 · 0 comments

Comments

@joepio
Copy link
Member

joepio commented Jan 20, 2023

Many common functions in atomic_lib, like Rersource::new, take a String as an argument.

This has some problems:

  • We need a lot of .into_string
  • Any functions relating to Subjects are hard to discover (e.g. check if URLs are valid)

We could improve this by:

  • Accepting impl From<String>, which deals with &str as well as String. This is the easy approach that already brings some decent DX improvements.
  • Accepting a new type for Subject, which implements From<String> and display.

This second Subject type deserves some thoughts:

  • Subjects are currently practically always http URLs. Therefore, it might make sense to simply treat them as URLs
  • We'll probably introduce a type of local-id in the future. There are (probably) still URLs!
  • We could add some useful features, like a method to check if it's a local or not, or what the root of the origin is or something.
  • If creating a Subject requires parsing the string, we should consider performance impact, e.g. when constructing a Resource from a binary blob. Same goes for serialisation! So perhaps URLs are a bit too expensive?
  • If we use one type for all these subjects, refactoring will be far easier (e.g. if we'll rename subject to id)
  • In a recent PR, we already have an AtomicURL type. This creates a URL under the hood, and provides some methods for getting specific routes / subdomains. Perhaps this could be the ID type?
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

No branches or pull requests

1 participant