henrib commented on code in PR #5882: URL: https://github.com/apache/hive/pull/5882#discussion_r2256738678
########## standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogFactory.java: ########## @@ -33,21 +33,23 @@ /** * Catalog & servlet factory. + * <p>This class is derivable on purpose; the factory class name is a configuration property, this class + * can serve as a base for specialization.</p> Review Comment: I hear and welcome your challenge 😃 ; let me try and further explain why I believe this 'pattern of extensible inheritable factory' is an acceptable compromise in this context. I actually embrace Mr Bloch rule phrased as 'favor composition over inheritance'; we are in a delineated case where we only want to allow real subtype of our factory. If this factory class were to become composable, this would imply we can delegate to an instance and call its methods from a user defined class instance; then all its methods should become public including its constructor. And this could lead towards exposing an interface, adding even more visibility and more complexity. If we switch back to private, no user defined class can use Hive's instance which will force anyone to duplicate (almost) all of its code to add specialization. Certainly a better alternative than the previous one at the expense of code duplication. The protected version allows surgical precision in extension. Since the catalog cache already delegates to the Catalog instance, it is a very attractive and convenient extension point. We could even make that more obvious by exposing a cache wrapping method. Let me know what you think. -- 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: gitbox-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
