Cleanup: Some simplifications in json_schema_compiler.

tools/json_schema_compiler/util_cc_helper.py

Index: tools/json_schema_compiler/util_cc_helper.py
diff --git a/tools/json_schema_compiler/util_cc_helper.py b/tools/json_schema_compiler/util_cc_helper.py
index 0e41abfa12b06c7d0b96bde0c5cbe6c67f7790b7..6549b4eab409ff46d6570ed410350825c1a3009f 100644
--- a/tools/json_schema_compiler/util_cc_helper.py
+++ b/tools/json_schema_compiler/util_cc_helper.py
@@ -36,17 +36,24 @@ class UtilCCHelper(object):
 
     return val % sub
 
-  def PopulateArrayFromList(self, src, dst, optional):
+  def PopulateArrayFromList(self, src, dst, optional, items_are_binary):

[not at google - send to devlin, 2015/01/13 18:15:14]

For boolean flags I prefer to make it explicit, like binary=False then forcing
callers to write binary=<expression>

[pneubeck (no reviews), 2015/01/14 10:54:15]

Not sure how you could /force/ the caller to use named arguments, as he can
always just pass in positional arguments and python will be happy, AFAIK.

Done.

     """Generates code to get an array from src into dst.
 
     src: ListValue*
     dst: std::vector or scoped_ptr<std::vector>
     """
+
+    binary_tag = 'ITEMS_ARE_NOT_BINARY';
+    if items_are_binary:
+      binary_tag = 'ITEMS_ARE_BINARY';
     if optional:
-      val = '%(namespace)s::PopulateOptionalArrayFromList(*%(src)s, &%(dst)s)'
+      val = ('%(namespace)s::PopulateOptionalArrayFromList'
+          '(*%(src)s, %(namespace)s::%(binary_tag)s(), &%(dst)s)')
     else:
-      val = '%(namespace)s::PopulateArrayFromList(*%(src)s, &%(dst)s)'
+      val = ('%(namespace)s::PopulateArrayFromList'
+          '(*%(src)s, %(namespace)s::%(binary_tag)s(), &%(dst)s)')

[not at google - send to devlin, 2015/01/13 18:15:14]

Given there's even more duplication, you should be able to sugarise it a little
more, like

optional_or_required = 'Optional' if optional else ''
binary_tag = 'ITEMS_ARE_BINARY' if binary else 'ITEMS_ARE_NOT_BINARY'
return '%s::Populate%sFromList(*%s), %s::%s(), &%s)' % ...

(I don't particularly like named string substitution params in these cases, just
obfuscates the code IMO)

[pneubeck (no reviews), 2015/01/14 10:54:15]

Slight variation of what you proposed because concatenating identifiers is IMO a
no-go as it destroys grep-ability.

     return val % {
+      'binary_tag': binary_tag,
       'namespace': _API_UTIL_NAMESPACE,
       'src': src,
       'dst': dst