okumin commented on code in PR #5882: URL: https://github.com/apache/hive/pull/5882#discussion_r2256420632
########## 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 got your point, and I respectfully have a small challenge 😃 . I have two points about why we can avoid `protected` here. First, I suppose most Java projects recommend using as restrictive accessibility[1] as possible, and Hive's source code follows the principle[2]. We need use cases when we use generous modifiers. Second, we can use not an inheritance but a composition when we really want to add some traits to `HMSCatalogFactory`; We will lose flexibility with `protected`. - [1] [Controlling Access to Members of a Class](https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html), Effective Java - [2] I understand we have some implementation that exceptionally abuses non-private modifiers -- 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
