gerrit approve: Cleanup option parsing to reduce unnecessary code

A lot of this code is just unnecessary complexity, instead we can
pass through the ApprovalType and save quite a few lines of code.

Change-Id: I02f51beeb35bf9e464c243bd0aa63336d9646dce
Signed-off-by: Shawn O. Pearce <sop@google.com>

2 files changed, 40 insertions, 68 deletions

diff --git a/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java b/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java
index 9019da716f..c2b3e8f274 100644
--- a/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java
+++ b/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java
@@ -49,7 +49,7 @@ public class ApproveCommand extends BaseCommand {
   @Override
   protected final CmdLineParser newCmdLineParser() {
     final CmdLineParser parser = super.newCmdLineParser();
-    for (CmdOption c : optionList) {
+    for (ApproveOption c : optionList) {
       parser.addOption(c, c);
     }
     return parser;
@@ -82,14 +82,14 @@ public class ApproveCommand extends BaseCommand {
   @Inject
   private FunctionState.Factory functionStateFactory;
 
-  private List<CmdOption> optionList;
+  private List<ApproveOption> optionList;
 
   @Override
   public final void start() {
     startThread(new CommandRunnable() {
       @Override
       public void run() throws Exception {
-        getApprovalNames();
+        initOptionList();
         parseCommandLine();
 
         final Transaction txn = db.beginTransaction();
@@ -113,9 +113,8 @@ public class ApproveCommand extends BaseCommand {
         sb.append(patchSetId.get());
         sb.append(": ");
 
-        for (CmdOption co : optionList) {
-          ApprovalCategory.Id category =
-              new ApprovalCategory.Id(co.approvalKey());
+        for (ApproveOption co : optionList) {
+          final ApprovalCategory.Id category = co.getCategoryId();
           PatchSetApproval.Key psaKey =
               new PatchSetApproval.Key(patchSetId, currentUser.getAccountId(),
                   category);
@@ -175,7 +174,7 @@ public class ApproveCommand extends BaseCommand {
   }
 
   private void addApproval(final PatchSetApproval.Key psaKey,
-      final Short score, final Change c, final CmdOption co,
+      final Short score, final Change c, final ApproveOption co,
       final Transaction txn) throws OrmException, UnloggedFailure {
     PatchSetApproval psa = db.patchSetApprovals().get(psaKey);
     boolean insert = false;
@@ -203,8 +202,8 @@ public class ApproveCommand extends BaseCommand {
     }
   }
 
-  private void getApprovalNames() {
-    optionList = new ArrayList<CmdOption>();
+  private void initOptionList() {
+    optionList = new ArrayList<ApproveOption>();
 
     for (ApprovalType type : approvalTypes.getApprovalTypes()) {
       String usage = "";
@@ -213,14 +212,13 @@ public class ApproveCommand extends BaseCommand {
 
       for (ApprovalCategoryValue v : type.getValues()) {
         usage +=
-            String.format("%3s", CmdOption.format(v.getValue())) + ": "
+            String.format("%3s", ApproveOption.format(v.getValue())) + ": "
                 + v.getName() + "\n";
       }
 
-      optionList.add(new CmdOption("--"
-          + category.getName().toLowerCase().replace(' ', '-'), usage, category
-          .getId().get(), type.getMin().getValue(), type.getMax().getValue(),
-          category.getName()));
+      final String name =
+          "--" + category.getName().toLowerCase().replace(' ', '-');
+      optionList.add(new ApproveOption(name, usage, type));
     }
   }
 
diff --git a/src/main/java/com/google/gerrit/server/ssh/commands/CmdOption.java b/src/main/java/com/google/gerrit/server/ssh/commands/ApproveOption.java
index a54a1c6072..c0b3def7f7 100644
--- a/src/main/java/com/google/gerrit/server/ssh/commands/CmdOption.java
+++ b/src/main/java/com/google/gerrit/server/ssh/commands/ApproveOption.java
@@ -14,6 +14,9 @@
 
 package com.google.gerrit.server.ssh.commands;
 
+import com.google.gerrit.client.data.ApprovalType;
+import com.google.gerrit.client.reviewdb.ApprovalCategory;
+
 import org.kohsuke.args4j.CmdLineException;
 import org.kohsuke.args4j.CmdLineParser;
 import org.kohsuke.args4j.Option;
@@ -24,91 +27,58 @@ import org.kohsuke.args4j.spi.Setter;
 
 import java.lang.annotation.Annotation;
 
-class CmdOption implements Option, Setter<Short> {
-  private String metaVar;
-  private boolean multiValued;
-  private String name;
-  private boolean required;
-  private String usage;
-
-  private String approvalKey;
-  private Short approvalMax;
-  private Short approvalMin;
-  private String descrName;
+final class ApproveOption implements Option, Setter<Short> {
+  private final String name;
+  private final String usage;
+  private final ApprovalType type;
 
   private Short value;
 
-  public CmdOption(final String name, final String usage, final String key,
-      final Short min, final Short max, final String descrName) {
+  ApproveOption(final String name, final String usage, final ApprovalType type) {
     this.name = name;
     this.usage = usage;
-
-    this.metaVar = "";
-    this.multiValued = false;
-    this.required = false;
-    this.value = null;
-
-    this.approvalKey = key;
-    this.approvalMax = max;
-    this.approvalMin = min;
-    this.descrName = descrName;
+    this.type = type;
   }
 
   @Override
-  public final String[] aliases() {
+  public String[] aliases() {
     return new String[0];
   }
 
   @Override
-  public final Class<? extends OptionHandler<Short>> handler() {
+  public Class<? extends OptionHandler<Short>> handler() {
     return Handler.class;
   }
 
   @Override
-  public final String metaVar() {
-    return metaVar;
+  public String metaVar() {
+    return "N";
   }
 
   @Override
-  public final boolean multiValued() {
-    return multiValued;
+  public boolean multiValued() {
+    return false;
   }
 
   @Override
-  public final String name() {
+  public String name() {
     return name;
   }
 
   @Override
-  public final boolean required() {
-    return required;
+  public boolean required() {
+    return false;
   }
 
   @Override
-  public final String usage() {
+  public String usage() {
     return usage;
   }
 
-  public final Short value() {
+  public Short value() {
     return value;
   }
 
-  public final String approvalKey() {
-    return approvalKey;
-  }
-
-  public final Short approvalMax() {
-    return approvalMax;
-  }
-
-  public final Short approvalMin() {
-    return approvalMin;
-  }
-
-  public final String descrName() {
-    return descrName;
-  }
-
   @Override
   public Class<? extends Annotation> annotationType() {
     return null;
@@ -129,13 +99,17 @@ class CmdOption implements Option, Setter<Short> {
     return false;
   }
 
+  ApprovalCategory.Id getCategoryId() {
+    return type.getCategory().getId();
+  }
+
   public static class Handler extends OneArgumentOptionHandler<Short> {
-    private final CmdOption cmdOption;
+    private final ApproveOption cmdOption;
 
     public Handler(final CmdLineParser parser, final OptionDef option,
         final Setter<Short> setter) {
       super(parser, option, setter);
-      this.cmdOption = (CmdOption) setter;
+      this.cmdOption = (ApproveOption) setter;
     }
 
     @Override
@@ -147,8 +121,8 @@ class CmdOption implements Option, Setter<Short> {
       }
 
       final short value = Short.parseShort(argument);
-      final short min = cmdOption.approvalMin;
-      final short max = cmdOption.approvalMax;
+      final short min = cmdOption.type.getMin().getValue();
+      final short max = cmdOption.type.getMax().getValue();
 
       if (value < min || value > max) {
         final String name = cmdOption.name();