Change TypeConverter<X,Y>::ConvertFrom and ConvertTo into a single symmetric

extensions/browser/api/serial/serial_api.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/browser/api/serial/serial_api.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/values.h"
@@ skipping 416 matching lines @@
 }
 
 }  // namespace core_api
 
 }  // namespace extensions
 
 namespace mojo {
 
 // static
 linked_ptr<extensions::core_api::serial::DeviceInfo>
-TypeConverter<device::serial::DeviceInfoPtr,
-              linked_ptr<extensions::core_api::serial::DeviceInfo> >::
-    ConvertTo(const device::serial::DeviceInfoPtr& device) {
+    TypeConverter<linked_ptr<extensions::core_api::serial::DeviceInfo>,
+                  device::serial::DeviceInfoPtr>>
+    ::Convert(const device::serial::DeviceInfoPtr& device) {

[darin (slow to review), 2014/08/27 03:30:37]

It seems like the second template argument might not be necessary. TypeConverter
could be defined like so:

  template <typename T>
  struct TypeConverter {
    static T Convert(const A& a);
    static T Convert(const B& b);
    static T Convert(const C& c);
  };

Another approach might be to rely on argument-dependent lookup so that these
converters wouldn't need to be in the mojo:: namespace. This would require the
return value to be an out param, like so:

  void Convert(const A& a, T* obj);
  void Convert(const B& b, T* obj);
  void Convert(const C& c, T* obj);

WDYT?

[Matt Perry, 2014/08/27 18:51:31]

I think the converters would still need to be in the mojo namespace so that
things like Array::From() still work.

We could potentially still have mojo::Convert(const A& a, T* obj) - e.g.
function overloading rather than template specializations. I think the drawback
there is that it's easier for the compiler to mess up which conversion function
is used (which would only really happen if there are two similar types). This
might happen if there are 2 similar Convert functions defined in different
translation units, and you include the wrong one.

Based on my reading of this article [1], it's safer to have template
specializations in a struct rather than a global function. I believe the same
reasoning would apply to your first example. (OTOH, I can't think of a concrete
example where this would matter, so maybe it's overly paranoid.)

[1]: http://www.gotw.ca/publications/mill17.htm

   linked_ptr<extensions::core_api::serial::DeviceInfo> info(
       new extensions::core_api::serial::DeviceInfo);
   info->path = device->path;
   if (device->has_vendor_id)
     info->vendor_id.reset(new int(static_cast<int>(device->vendor_id)));
   if (device->has_product_id)
     info->product_id.reset(new int(static_cast<int>(device->product_id)));
   if (device->display_name)
     info->display_name.reset(new std::string(device->display_name));
   return info;
 }
 
 }  // namespace mojo