KnightChess commented on code in PR #11408:
URL: https://github.com/apache/hudi/pull/11408#discussion_r1630464622
##########
hudi-common/src/main/java/org/apache/hudi/internal/schema/visitor/NameToPositionVisitor.java:
##########
@@ -34,65 +30,36 @@
/**
* Schema visitor to produce name -> position map for internalSchema.
Positions are assigned in a depth-first manner.
*/
-public class NameToPositionVisitor extends InternalSchemaVisitor<Map<String,
Integer>> {
- private final Deque<String> fieldNames = new LinkedList<>();
- private final Map<String, Integer> nameToId = new HashMap<>();
+public class NameToPositionVisitor extends AbstractNameVisitor<Map<String,
Integer>> {
private final AtomicInteger position = new AtomicInteger(0);
+ public NameToPositionVisitor() {
+ super(new HashMap<>());
+ }
@Override
public void beforeField(Types.Field field) {
nameToId.put(createFullName(field.name(), fieldNames),
position.getAndIncrement());
fieldNames.push(field.name());
}
- @Override
- public void afterField(Types.Field field) {
Review Comment:
This will cause code to read a little difficulty.
In the original implementation, can read the `afterField` after
`beforeField`. now, if I am author for this code, when I update the code, I
know there has one method is `afterField` also need to change, but for other
developer, may lose this method.
And has this problem when reading the code too, hese methods are not
suitable for independent abstraction in a sense
--
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]
