Reorganized OFFactories
diff --git a/java_gen/codegen.py b/java_gen/codegen.py
index bc9b9a0..1e68acd 100644
--- a/java_gen/codegen.py
+++ b/java_gen/codegen.py
@@ -147,11 +147,11 @@
                             test_data=unit_test.test_data)
 
     def create_of_factories(self):
-        factory = self.java_model.of_factory
-        self.render_class(clazz=factory, template="of_factory_interface.java", factory=factory)
-        for factory_class in factory.factory_classes:
-            self.render_class(clazz=factory_class, template="of_factory_class.java", factory=factory_class, model=self.java_model)
-        self.render_class(clazz=java_model.OFGenericClass(package="org.openflow.protocol", name="OFFactories"), template="of_factories.java", versions=self.java_model.versions)
+        for factory in self.java_model.of_factories:
+            self.render_class(clazz=factory, template="of_factory_interface.java", factory=factory)
+            for factory_class in factory.factory_classes:
+                self.render_class(clazz=factory_class, template="of_factory_class.java", factory=factory_class, model=self.java_model)
+            self.render_class(clazz=java_model.OFGenericClass(package="org.openflow.protocol", name="OFFactories"), template="of_factories.java", versions=self.java_model.versions)
 
 def copy_prewrite_tree(basedir):
     """ Recursively copy the directory structure from ./java_gen/pre-write
diff --git a/java_gen/java_model.py b/java_gen/java_model.py
index 0fefadb..d7f7370 100644
--- a/java_gen/java_model.py
+++ b/java_gen/java_model.py
@@ -44,6 +44,7 @@
 import test_data
 
 import java_gen.java_type as java_type
+from java_gen.java_type import erase_type_annotation
 
 class JavaModel(object):
     enum_blacklist = set(("OFDefinitions",))
@@ -83,6 +84,10 @@
 
         return interfaces
 
+    @memoize
+    def interface_by_name(self, name):
+        return find(lambda i: erase_type_annotation(i.name) == erase_type_annotation(name), self.interfaces)
+
     @property
     @memoize
     def all_classes(self):
@@ -114,11 +119,40 @@
 
     @property
     @memoize
-    def of_factory(self):
-           return OFFactory(
-                    package="org.openflow.protocol",
+    def of_factories(self):
+        prefix = "org.openflow.protocol"
+
+        factories = OrderedDict()
+
+        sub_factory_classes = ("OFAction", "OFInstruction", "OFMeterBand", "OFOxm", "OFQueueProp")
+        for base_class in sub_factory_classes:
+            package = base_class[2:].lower()
+            remove_prefix = base_class[2].lower() + base_class[3:]
+
+            # HACK need to have a better way to deal with parameterized base classes
+            annotated_base_class = base_class + "<?>" if base_class == "OFOxm" else base_class
+
+            factories[base_class] = OFFactory(package="%s.%s" % (prefix, package),
+                    name=base_class + "s", members=[], remove_prefix=remove_prefix, base_class=annotated_base_class, sub_factories={})
+
+        factories[""] = OFFactory(
+                    package=prefix,
                     name="OFFactory",
-                    members=self.interfaces)
+                    remove_prefix="",
+                    members=[], base_class="OFMessage", sub_factories=OrderedDict(
+                        ("{}{}s".format(n[2].lower(), n[3:]), "{}s".format(n)) for n in sub_factory_classes ))
+
+        for i in self.interfaces:
+            for n, factory in factories.items():
+                if n == "":
+                    factory.members.append(i)
+                    break
+                else:
+                    super_class = self.interface_by_name(n)
+                    if i.is_instance_of(super_class):
+                        factory.members.append(i)
+                        break
+        return factories.values()
 
     def generate_class(self, clazz):
         """ return wether or not to generate implementation class clazz.
@@ -142,19 +176,39 @@
             return True
 
 
-class OFFactory(namedtuple("OFFactory", ("package", "name", "members"))):
+class OFFactory(namedtuple("OFFactory", ("package", "name", "members", "remove_prefix", "base_class", "sub_factories"))):
     @property
     def factory_classes(self):
             return [ OFFactoryClass(
                     package="org.openflow.protocol.ver{}".format(version.of_version),
-                    name="OFFactoryVer{}".format(version.of_version),
+                    name="{}Ver{}".format(self.name, version.of_version),
                     interface=self,
                     version=version
                     ) for version in model.versions ]
 
+    def method_name(self, member, builder=True):
+        n = member.variable_name
+        if n.startswith(self.remove_prefix):
+            n = n[len(self.remove_prefix):]
+            n = n[0].lower() + n[1:]
+        if builder:
+            return "build" + n[0].upper() + n[1:]
+        else:
+            return n
 
 OFGenericClass = namedtuple("OFGenericClass", ("package", "name"))
-OFFactoryClass = namedtuple("OFFactory", ("package", "name", "interface", "version"))
+class OFFactoryClass(namedtuple("OFFactoryClass", ("package", "name", "interface", "version"))):
+    @property
+    def base_class(self):
+        return self.interface.base_class
+
+    @property
+    def versioned_base_class(self):
+        base_class_interface = model.interface_by_name(self.interface.base_class)
+        if base_class_interface and base_class_interface.has_version(self.version):
+            return base_class_interface.versioned_class(self.version)
+        else:
+            return None
 
 model = JavaModel()
 
diff --git a/java_gen/templates/_imports.java b/java_gen/templates/_imports.java
index 5b86571..a4c564c 100644
--- a/java_gen/templates/_imports.java
+++ b/java_gen/templates/_imports.java
@@ -1,6 +1,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import org.openflow.protocol.*;
 import org.openflow.protocol.action.*;
 import org.openflow.protocol.meterband.*;
diff --git a/java_gen/templates/of_factories.java b/java_gen/templates/of_factories.java
index cce134d..9225510 100644
--- a/java_gen/templates/of_factories.java
+++ b/java_gen/templates/of_factories.java
@@ -40,7 +40,7 @@
         switch(version) {
             //:: for v in versions:
             case ${v.constant_version}:
-                return org.openflow.protocol.ver${v.of_version}.OFFactoryVer${v.of_version}.getInstance();
+                return org.openflow.protocol.ver${v.of_version}.OFFactoryVer${v.of_version}.INSTANCE;
             //:: #endfor
             default:
                 throw new IllegalArgumentException("Unknown version: "+version);
diff --git a/java_gen/templates/of_factory_class.java b/java_gen/templates/of_factory_class.java
index fec0bcf..e83344b 100644
--- a/java_gen/templates/of_factory_class.java
+++ b/java_gen/templates/of_factory_class.java
@@ -36,25 +36,53 @@
 //:: include("_imports.java")
 
 public class ${factory.name} implements ${factory.interface.name} {
+    public final static ${factory.name} INSTANCE = new ${factory.name}();
+    private ${factory.name}() {}
+
+    //:: for name, clazz in factory.interface.sub_factories.items():
+    public ${clazz} ${name}() {
+        return ${clazz}Ver${factory.version.of_version}.INSTANCE;
+    }
+    //:: #endfor
+
 //:: for i in factory.interface.members:
     //:: if i.is_virtual:
     //::    continue
     //:: #endif
 
-//::   if i.has_version(factory.version) and model.generate_class(i.versioned_class(factory.version)):
-    public ${i.name}.Builder create${i.name[2:]}Builder() {
+    //:: if len(i.writeable_members) > 0:
+    public ${i.name}.Builder ${factory.interface.method_name(i, builder=True)}() {
+        //::   if i.has_version(factory.version) and model.generate_class(i.versioned_class(factory.version)):
         return new ${i.versioned_class(factory.version).name}.Builder();
-    }
-//:: else:
-    public ${i.name}.Builder create${i.name[2:]}Builder() throws UnsupportedOperationException {
+        //:: else:
         throw new UnsupportedOperationException("${i.name} not supported in version ${factory.version}");
+        //:: #endif
     }
-//:: #endif
+    //:: #endif
+    //:: if len(i.writeable_members) <= 2:
+    public ${i.name} ${factory.interface.method_name(i, builder=False)}(${", ".join("%s %s" % (p.java_type.public_type, p.name) for p in i.writeable_members)}) {
+        //::   if i.has_version(factory.version) and model.generate_class(i.versioned_class(factory.version)):
+        //:: if len(i.writeable_members) > 0:
+        return new ${i.versioned_class(factory.version).name}(
+                ${",\n                      ".join(
+                         [ prop.name for prop in i.versioned_class(factory.version).data_members])}
+                    );
+        //:: else:
+        return ${i.versioned_class(factory.version).name}.INSTANCE;
+        //:: #endif
+        //:: else:
+        throw new UnsupportedOperationException("${i.name} not supported in version ${factory.version}");
+        //:: #endif
+    }
+    //:: #endif
 //:: #endfor
 
-    public OFMessageReader<OFMessage> getMessageReader() {
-        return OFMessageVer${factory.version.of_version}.READER;
+    public OFMessageReader<${factory.base_class}> getReader() {
+//:: if factory.versioned_base_class:
+        return ${factory.versioned_base_class.name}.READER;
+//:: else:
+        throw new UnsupportedOperationException("Reader<${factory.base_class}> not supported in version ${factory.version}");
+//:: #endif
     }
 
-    //:: include("_singleton.java", msg=factory)
 }
diff --git a/java_gen/templates/of_factory_interface.java b/java_gen/templates/of_factory_interface.java
index 39432a8..fcb690f 100644
--- a/java_gen/templates/of_factory_interface.java
+++ b/java_gen/templates/of_factory_interface.java
@@ -31,17 +31,27 @@
 
 //:: include('_autogen.java')
 
-package org.openflow.protocol;
+package ${factory.package};
 
 //:: include("_imports.java")
 
 public interface ${factory.name} {
+    // Subfactories
+//:: for name, clazz in factory.sub_factories.items():
+    ${clazz} ${name}();
+//:: #endfor
+
 //:: for i in factory.members:
     //:: if i.is_virtual:
     //::    continue
     //:: #endif
-    ${i.name}.Builder create${i.name[2:]}Builder()${ "" if i.is_universal else " throws UnsupportedOperationException"};
+    //:: if len(i.writeable_members) > 0:
+    ${i.name}.Builder ${factory.method_name(i, builder=True)}()${ "" if i.is_universal else " throws UnsupportedOperationException"};
+    //:: #endif
+    //:: if len(i.writeable_members) <= 2:
+    ${i.name} ${factory.method_name(i, builder=False )}(${", ".join("%s %s" % (p.java_type.public_type, p.name) for p in i.writeable_members)});
+    //:: #endif
 //:: #endfor
 
-    OFMessageReader<OFMessage> getMessageReader();
+    OFMessageReader<${factory.base_class}> getReader();
 }
diff --git a/java_gen/templates/unit_test.java b/java_gen/templates/unit_test.java
index 45472f2..47dcc24 100644
--- a/java_gen/templates/unit_test.java
+++ b/java_gen/templates/unit_test.java
@@ -55,9 +55,9 @@
     //:: if "java" in test_data:
     @Test
     public void testWrite() {
-        ${var_type}.Builder builder = factory.create${var_type[2:]}Builder();
+        ${var_type}.Builder builder = factory.build${var_type[2:]}();
         ${test_data["java"]};
-        ${var_type} ${var_name} = builder.getMessage();
+        ${var_type} ${var_name} = builder.build();
         ChannelBuffer bb = ChannelBuffers.dynamicBuffer();
         ${var_name}.writeTo(bb);
         byte[] written = new byte[bb.readableBytes()];
@@ -68,9 +68,9 @@
 
     @Test
     public void testRead() throws Exception {
-        ${var_type}.Builder builder = factory.create${var_type[2:]}Builder();
+        ${var_type}.Builder builder = factory.build${var_type[2:]}();
         ${test_data["java"]};
-        ${var_type} ${var_name}Built = builder.getMessage();
+        ${var_type} ${var_name}Built = builder.build();
 
         ChannelBuffer input = ChannelBuffers.copiedBuffer(${msg.constant_name}_SERIALIZED);
 
diff --git a/test_data/of13/flow_add.data b/test_data/of13/flow_add.data
index d01c209..10b3d2f 100644
--- a/test_data/of13/flow_add.data
+++ b/test_data/of13/flow_add.data
@@ -62,11 +62,11 @@
     .setOutPort(OFPort.of(6))
     .setOutGroup(8)
     .setFlags(0)
-    .setMatch(factory.createMatchV3Builder().getMessage()) // FIXME: @yotam: replace once we have generic ofmatch
+    .setMatch(factory.buildMatchV3().build()) // FIXME: @yotam: replace once we have generic ofmatch
     .setInstructions(
         ImmutableList.<OFInstruction>of(
-            factory.createInstructionGotoTableBuilder().setTableId((byte) 4).getMessage(),
-            factory.createInstructionGotoTableBuilder().setTableId((byte) 7).getMessage()
+            factory.instructions().gotoTable((short) 4),
+            factory.instructions().gotoTable((short) 7)
         )
     );
 
diff --git a/test_data/of13/packet_in.data b/test_data/of13/packet_in.data
index 71e17b1..951f212 100644
--- a/test_data/of13/packet_in.data
+++ b/test_data/of13/packet_in.data
@@ -30,6 +30,8 @@
         ofp.oxm.in_port_masked(value=4, value_mask=5)]),
     data="abc")
 -- java
+OFOxms oxms = factory.oxms();
+
 builder
    .setXid(0x12345678)
    .setBufferId(100)
@@ -38,12 +40,12 @@
    .setTableId((byte) 20)
    .setCookie(U64.parseHex("FEDCBA9876543210"))
    .setMatch(
-        factory.createMatchV3Builder().setOxmList(
-            ImmutableList.of(
-                factory.createOxmArpOpBuilder().setValue(1).getMessage(),
-                factory.createOxmInPortMaskedBuilder().setValue(OFPort.of(4)).setValueMask(OFPort.of(5)).getMessage()
+        factory.buildMatchV3().setOxmList(
+            OFOxmList.of(
+                oxms.arpOp(ArpOpcode.ARP_OPCODE_REQUEST),
+                oxms.inPortMasked(OFPort.of(4), OFPort.of(5))
             )
-        ).getMessage()
+        ).build()
     )
     .setData(new byte[] { 97, 98, 99 } );