dimas-b commented on PR #741: URL: https://github.com/apache/polaris/pull/741#issuecomment-2598994163
I'll add my 2 cents, since I commented above saying that the renaming made sense :) It's true that `RealmId` by itself is not used as the realm's identifier in code. However, the real identifier is a `String`, so to add type safety and simplify usage searches (in IDEs), I still think using a dedicated "holder" class for that ID makes sense. As to `Realm` vs. `RealmId`, I think the former implies that the realm's data or content is held by the object, which is not the case. Only the ID is held inside that object. Now, I see that `RealmId.id` may look awkward. I think it would also be ok for follow-up PRs to rename that. I'm personally ok with `RealmId.id()`, but `RealmId.asString()` would also work from my POV (but it's more verbose). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
