new code layout for parameter initializers

Implemented the smartest version described in http://blog.jangaroo.net/2012/01/simulating-actionscript-parameter.html Added a simple wrapper around java.text.MessageFormat because the original sucks. Maybe we should even use Freemarker for code generation?
CoreMedia · Feb 9, 2012 · 18cdf3d · 18cdf3d
1 parent 3d30565
commit 18cdf3d
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 18 deletions.
diff --git a/jangaroo/jangaroo-compiler/src/main/java/net/jangaroo/jooc/backend/JsCodeGenerator.java b/jangaroo/jangaroo-compiler/src/main/java/net/jangaroo/jooc/backend/JsCodeGenerator.java
@@ -77,7 +77,9 @@
 
 import java.io.File;
 import java.io.IOException;
+import net.jangaroo.jooc.util.MessageFormat;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -374,40 +376,70 @@ public CodeGenerator getParameterInitializerCodeGenerator(final Parameters param
     return new CodeGenerator() {
       @Override
       public void generate(JsWriter out, boolean first) throws IOException {
-        // first pass: generate conditionals and count parameters.
+        // collect the ... (rest) parameter and all optional parameters with their position index:
-        int cnt = 0;
+        int restParamIndex = -1;
-        StringBuilder code = new StringBuilder();
+        Parameter restParam = null;
-        for (Parameters parameters = params; parameters != null; parameters = parameters.getTail()) {
+        Map<Integer,Parameter> paramByIndex = new HashMap<Integer, Parameter>();
+        Parameters parameters = params;
+        for (int paramIndex = 0; parameters != null; parameters = parameters.getTail()) {
           Parameter param = parameters.getHead();
           if (param.isRest()) {
+            restParamIndex = paramIndex;
+            restParam = param;
             break;
           }
           if (param.hasInitializer()) {
-            code.insert(0, "if(arguments.length<" + (cnt + 1) + "){");
+            paramByIndex.put(paramIndex, param);
           }
-          ++cnt;
+          ++paramIndex;
         }
-        out.write(code.toString());
+        generateParameterInitializers(out, paramByIndex);
-        // second pass: generate initializers and rest param code.
+        if (restParam != null) {
-        for (Parameters parameters = params; parameters != null; parameters = parameters.getTail()) {
+          generateRestParamCode(restParam, restParamIndex);
-          Parameter param = parameters.getHead();
+        }
-          if (param.isRest()) {
+      }
-            generateRestParamCode(param, cnt);
+
+    };
+  }
+
+  private static final MessageFormat IF_ARGUMENT_LENGTH_LTE_$N = new MessageFormat("if(arguments.length<={0})");
+  private static final MessageFormat SWITCH_$INDEX = new MessageFormat("switch({0,choice,0#arguments.length|0<Math.max(arguments.length,{0})})");
+  private static final MessageFormat CASE_$N = new MessageFormat("case {0}:");
+
+  private void generateParameterInitializers(JsWriter out, Map<Integer, Parameter> paramByIndex) throws IOException {
+    Iterator<Map.Entry<Integer, Parameter>> paramByIndexIterator = paramByIndex.entrySet().iterator();
+    if (paramByIndexIterator.hasNext()) {
+      Map.Entry<Integer, Parameter> indexAndParam = paramByIndexIterator.next();
+      Integer firstParamIndex = indexAndParam.getKey();
+      if (!paramByIndexIterator.hasNext()) {
+        // only one parameter initializer: use "if"
+        out.write(IF_ARGUMENT_LENGTH_LTE_$N.format(firstParamIndex));
+        generateBodyInitializerCode(indexAndParam.getValue());
+      } else {
+        // more than one parameter initializer: use "switch"
+        out.write(SWITCH_$INDEX.format(firstParamIndex));
+        out.write("{");
+
+        while (true) {
+          out.write(CASE_$N.format(indexAndParam.getKey()));
+          generateBodyInitializerCode(indexAndParam.getValue());
+          if (!paramByIndexIterator.hasNext()) {
             break;
           }
-          if (param.hasInitializer()) {
+          indexAndParam = paramByIndexIterator.next();
-            generateBodyInitializerCode(param);
-            out.write("}");
-          }
         }
+
+        out.write("}");
       }
-    };
+    }
   }
 
+  private static final MessageFormat VAR_$NAME_EQUALS_ARGUMENTS_SLICE_$INDEX = new MessageFormat("var {0}=Array.prototype.slice.call(arguments{1,choice,0#|0<,{1}});");
+
   public void generateRestParamCode(Parameter param, int paramIndex) throws IOException {
     String paramName = param.getName();
     if (paramName != null && !(paramName.equals("arguments") && paramIndex == 0)) {
-      out.write("var " + paramName + "=Array.prototype.slice.call(arguments" + (paramIndex == 0 ? "" : "," + paramIndex) + ");");
+      out.write(VAR_$NAME_EQUALS_ARGUMENTS_SLICE_$INDEX.format(paramName, paramIndex));
     }
   }
 

diff --git a/jangaroo/jangaroo-compiler/src/main/java/net/jangaroo/jooc/util/MessageFormat.java b/jangaroo/jangaroo-compiler/src/main/java/net/jangaroo/jooc/util/MessageFormat.java
@@ -0,0 +1,24 @@
+package net.jangaroo.jooc.util;
+
+/**
+ * A better {@link java.text.MessageFormat} that takes an arbitrary number of parameters.
+ * This is to avoid a pitfall in the original MessageFormat: if you call {@link java.text.MessageFormat#format(Object)}
+ * you'd assume that the parameter is a single argument. Instead, it has to be an <code>Object[]</code> like
+ * in the old times. This is unintuitive, as {@link java.text.MessageFormat#format(String, Object...)}
+ * works in the modern way.
+ *
+ * @see java.text.MessageFormat
+ */
+public class MessageFormat {
+
+  private java.text.MessageFormat messageFormat;
+
+  public MessageFormat(String pattern) {
+    messageFormat = new java.text.MessageFormat(pattern);
+  }
+
+  public String format(Object ...arguments) {
+    return messageFormat.format(arguments);
+  }
+
+}