cshannon commented on code in PR #3401:
URL: https://github.com/apache/accumulo/pull/3401#discussion_r1198885348
##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -155,4 +142,9 @@ public int hashCode() {
public String toString() {
return normalizedPath;
}
+
+ public static TabletFile of(final Path path) {
Review Comment:
In this particular case it could be removed as it doesn't buy that much,
this for me is mostly a personal preference. If we keep it we could definitely
make the constructor private if we want to enforce the new method. I have grown
to like using more descriptive and fluent apis ever since lambdas were
introduced and the builder pattern became more of a thing. There's 2 reasons I
like using these types of methods vs a constructor directly.
1. I personally just find it is nice to create static initializer methods to
make things more compact. It's just nicer to write `TabletFile.of(path) `vs
having to write `new TabletFile(path)` .
2. Constructors are not very descriptive which is annoying. If you have 10
different constructors they are all called the same thing (class name) with
different arguments so it's not always clear the difference at first glance. By
using static methods you can name them anything you want and it makes it very
clear in the code what you are doing. A good example of this being beneficial
was my PR for #3257 which added a bunch of methods for Location
[here](https://github.com/apache/accumulo/blob/4d7f81954be05e414f814aa116ed65536edf7ba6/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java#L204-L226).
It makes the code nice to just be able to say `Location.last(instance)` or
`Location.current(instance)` etc without having to call a constructor directly
and pass in more args.
--
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]
