justinmclean commented on code in PR #5688:
URL: https://github.com/apache/gravitino/pull/5688#discussion_r1862925410
##########
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##########
@@ -140,41 +145,78 @@ private void handleMetalakeCommand() {
String url = getUrl();
FullName name = new FullName(line);
String metalake = name.getMetalakeName();
-
- if (CommandActions.DETAILS.equals(command)) {
- if (line.hasOption(GravitinoOptions.AUDIT)) {
- newMetalakeAudit(url, ignore, metalake).handle();
- } else {
- newMetalakeDetails(url, ignore, metalake).handle();
- }
- } else if (CommandActions.LIST.equals(command)) {
- newListMetalakes(url, ignore).handle();
- } else if (CommandActions.CREATE.equals(command)) {
- String comment = line.getOptionValue(GravitinoOptions.COMMENT);
- newCreateMetalake(url, ignore, metalake, comment).handle();
- } else if (CommandActions.DELETE.equals(command)) {
- boolean force = line.hasOption(GravitinoOptions.FORCE);
- newDeleteMetalake(url, ignore, force, metalake).handle();
- } else if (CommandActions.SET.equals(command)) {
- String property = line.getOptionValue(GravitinoOptions.PROPERTY);
- String value = line.getOptionValue(GravitinoOptions.VALUE);
- newSetMetalakeProperty(url, ignore, metalake, property, value).handle();
- } else if (CommandActions.REMOVE.equals(command)) {
- String property = line.getOptionValue(GravitinoOptions.PROPERTY);
- newRemoveMetalakeProperty(url, ignore, metalake, property).handle();
- } else if (CommandActions.PROPERTIES.equals(command)) {
- newListMetalakeProperties(url, ignore, metalake).handle();
- } else if (CommandActions.UPDATE.equals(command)) {
- if (line.hasOption(GravitinoOptions.COMMENT)) {
- String comment = line.getOptionValue(GravitinoOptions.COMMENT);
- newUpdateMetalakeComment(url, ignore, metalake, comment).handle();
- }
- if (line.hasOption(GravitinoOptions.RENAME)) {
- String newName = line.getOptionValue(GravitinoOptions.RENAME);
- boolean force = line.hasOption(GravitinoOptions.FORCE);
- newUpdateMetalakeName(url, ignore, force, metalake, newName).handle();
+ Map<Set<String>, Runnable> commandMap = new HashMap<>();
+
+ commandMap.put(
+ Sets.newHashSet(CommandActions.DETAILS),
+ () -> {
+ newMetalakeDetails(url, ignore, metalake).handle();
+ });
+ commandMap.put(
+ Sets.newHashSet(CommandActions.DETAILS, GravitinoOptions.AUDIT),
+ () -> {
+ newMetalakeAudit(url, ignore, metalake).handle();
+ });
+ commandMap.put(
+ Sets.newHashSet(CommandActions.LIST), () -> newListMetalakes(url,
ignore).handle());
+ commandMap.put(
+ Sets.newHashSet(CommandActions.CREATE),
+ () -> {
+ String comment = line.getOptionValue(GravitinoOptions.COMMENT);
+ newCreateMetalake(url, ignore, metalake, comment).handle();
+ });
+ commandMap.put(
+ Sets.newHashSet(CommandActions.DELETE),
+ () -> {
+ boolean force = line.hasOption(GravitinoOptions.FORCE);
+ newDeleteMetalake(url, ignore, force, metalake).handle();
+ });
+ commandMap.put(
+ Sets.newHashSet(CommandActions.SET, GravitinoOptions.PROPERTY,
GravitinoOptions.VALUE),
+ () -> {
+ String property = line.getOptionValue(GravitinoOptions.PROPERTY);
+ String value = line.getOptionValue(GravitinoOptions.VALUE);
+ newSetMetalakeProperty(url, ignore, metalake, property,
value).handle();
+ });
+ commandMap.put(
+ Sets.newHashSet(CommandActions.REMOVE, GravitinoOptions.PROPERTY),
+ () -> {
+ String property = line.getOptionValue(GravitinoOptions.PROPERTY);
+ newRemoveMetalakeProperty(url, ignore, metalake, property).handle();
+ });
+ commandMap.put(
+ Sets.newHashSet(CommandActions.PROPERTIES),
+ () -> newListMetalakeProperties(url, ignore, metalake).handle());
+ commandMap.put(
+ Sets.newHashSet(CommandActions.UPDATE, GravitinoOptions.COMMENT),
+ () -> {
+ String comment = line.getOptionValue(GravitinoOptions.COMMENT);
+ newUpdateMetalakeComment(url, ignore, metalake, comment).handle();
+ });
+ commandMap.put(
+ Sets.newHashSet(CommandActions.UPDATE, GravitinoOptions.RENAME),
+ () -> {
+ String newName = line.getOptionValue(GravitinoOptions.RENAME);
+ boolean force = line.hasOption(GravitinoOptions.FORCE);
+ newUpdateMetalakeName(url, ignore, force, metalake,
newName).handle();
+ });
Review Comment:
You can ignore the build error, as I said above. This solution has no
performance benefit, but that doesn't matter as a user will not notice a few
milliseconds. There is a benefit in that we have exact matching with the set of
valid commands, and it gets around some limitations of if/else style coding.
Changing to a switch/case would just be a cosmetic change, but it can be
done; again, for this, the performance isn't an issue and would be of
negligible difference. Note that in some cases, if/else is faster than a
switch, especially if some options are more common than others as they are here.
--
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]
