jnaous commented on a change in pull request #9111: Add HashJoinSegment, a
virtual segment for joins.
URL: https://github.com/apache/druid/pull/9111#discussion_r365277229
##########
File path:
processing/src/main/java/org/apache/druid/query/lookup/LookupExtractor.java
##########
@@ -100,12 +100,29 @@
return map;
}
+ /**
+ * Returns true if this lookup extractor's {@link #iterable()} method will
return a valid iterator.
+ */
+ public boolean canIterate()
+ {
+ return false;
+ }
+
+ /**
+ * Returns an Iterable that iterates over the keys and values in this lookup
extractor.
+ *
+ * @throws UnsupportedOperationException if {@link #canIterate()} returns
false.
+ */
+ public Iterable<Map.Entry<String, String>> iterable()
+ {
+ throw new UnsupportedOperationException("Cannot iterate");
+ }
+
Review comment:
One of the reasons I dislike class hierarchies and default methods is that
they define the behavior of the subclasses. Which means that if you change the
behavior of the parent class, you're impacting the behavior of the subclasses,
and if you don't have unit tests that ensure that subclasses have the expected
default behavior inherited from the parent, you might have subtle errors that
show up in the future. I'm a big proponent of composition rather than
inheritance, which is why Go abandoned class hierarchies. I would prefer if the
methods were abstract instead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
