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

Attempt to use common trait between variants #207

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michalfita
Copy link
Contributor

Important

This is in the ideation stage, not supposed to be merged without any real life proof of sense.

I made attempt to create a trait shared between lossy and lossless variants of Repository, but this has following flaws:

  • Interface over lossy variant works nice as we can refer to member; I tried Cow<> but now dealing with fact Paragraph::get() may fail become a problem (unwrap() is plain wrong).
  • Use Cow<> is going to work for some cases, but may be a problem for some others, especially if wrapped in Option<>
  • For optional fields use of .ok() turns potential errors of late parsing into None, I have doubts if it's acceptable.

Problems with lossless structure:

  • Paragraphs are copied out of Deb822, there's not reference to them
  • If lossless::Repositories store Deb822, I can only create lossless::Repository from these paragraphs, there's not way to return reference to them (that's different than in lossy variant).

Maybe I'm heading in wrong direction (no modifications/setting taken into account yet), as this route gives me identical API of both variants, but with problems. Maybe the role of lossless variant should be to produce out lossy variant and accept one to perform modifications.

Can we discuss pros and cons?

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.

1 participant