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

UniqueId.fromFirebaseId factory should not be in the core domain layer #3

Closed
fabriziocacicia opened this issue Mar 18, 2020 · 3 comments

Comments

@fabriziocacicia
Copy link

I think that this factory should not exist in the core domain layer since it injects the knowledge of Firebase in that layer, which should be completely agnostic to specific technologic choices.

As I already commented here, this could be avoided by using a different approach in structuring the project.
A per-feature splitting would let to insert this UniqueId.fromFirebaseId factory in the domain layer of the auth "package" so to say, imagining to have a FirebaseUniqueId object that inherits a common/core generic UniqueId one.

@ResoDev
Copy link
Contributor

ResoDev commented Apr 16, 2020

The naming of this factory is quite unfortunate. I should really fix it to not be Firebase-specific, but even now, this factory logic doesn't have anything to do with Firebase. It simply takes any String and trusts that it's unique. Perhaps renaming the factory to fromIdString would do?

@fabriziocacicia
Copy link
Author

fabriziocacicia commented Apr 16, 2020

The purpose of this factory should be to create a UniqueId object from a string that we trust and we know is already unique.
So, maybe you could call the factory UniqueId.fromTrustedString or UniqueId.fromUniqueString.

@ResoDev
Copy link
Contributor

ResoDev commented Apr 19, 2020

Thank you, fixed.

@ResoDev ResoDev closed this as completed Apr 19, 2020
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

2 participants