Create a nicer interface for base::mac::GetValueFromDictionary

base/mac/foundation_util.mm

Index: base/mac/foundation_util.mm
diff --git a/base/mac/foundation_util.mm b/base/mac/foundation_util.mm
index 8a4ce471765e1157886e293ae4c7b9007ccc24f7..777836ae9c5eaec094a29dd0c8234923983a4762 100644
--- a/base/mac/foundation_util.mm
+++ b/base/mac/foundation_util.mm
@@ -348,6 +348,41 @@ CF_CAST_DEFN(CFNumber);
 CF_CAST_DEFN(CFSet);
 CF_CAST_DEFN(CFString);
 
+#define GET_VAL_FROM_DICT_DEFN(TypeCF) \

[Mark Mentovai, 2011/11/11 23:20:45]

No reason to abbreviate here, you can write GET_VALUE_FROM_DICTIONARY_DEFN.

+template<> TypeCF##Ref \
+GetValueFromDictionary<TypeCF##Ref>(CFDictionaryRef dict, CFStringRef key) { \
+  CFTypeRef val = CFDictionaryGetValue(dict, key); \

[Mark Mentovai, 2011/11/11 23:20:45]

val and value as names are confusing.

+  TypeCF##Ref value = base::mac::CFCast<TypeCF##Ref>(val); \

[Mark Mentovai, 2011/11/11 23:20:45]

base::mac:: isn’t necessary, you’re already in that namespace.

+  if (!val) \
+    return value; \
+  if (!value) { \

[Mark Mentovai, 2011/11/11 23:20:45]

The contents of this block should be pulled out into its own
anonymous-namespaced helper function, so you’d write:

  if (!value) { \
    DLOG(WARNING) <<
        GetValueFromDictionaryErrorMessage(key, TypeCF##GetTypeID(), val); \
  } \
  return value; \

Why? Avoidance of duplication. You can write the function that produces the
error string in a totally type-independent way. You wind up only emitting one
copy of that code instead of ten, one for each flavor of this function that you
currently output.

+    ScopedCFTypeRef<CFStringRef> expected_type_ref( \
+        CFCopyTypeIDDescription(TypeCF##GetTypeID())); \
+    ScopedCFTypeRef<CFStringRef> actual_type_ref( \
+        CFCopyTypeIDDescription(CFGetTypeID(val))); \
+    DLOG(WARNING) << "Expected value for key " \
+                 << base::SysCFStringRefToUTF8(key) \

[Mark Mentovai, 2011/11/11 23:20:45]

The alignment’s all off here.

It’s off in the spot where you copied this from, too. Oops.

+                 << " to be " \
+                 << base::SysCFStringRefToUTF8(expected_type_ref) \
+                 << " but it was " \
+                 << base::SysCFStringRefToUTF8(actual_type_ref) \
+                 << " instead"; \
+    return NULL; \
+  } \
+  return value; \
+}

[Mark Mentovai, 2011/11/11 23:20:45]

You could write this entire function without any macros by putting something
like this into the header:

template<typename CFT>
CFT GetValueFromDictionary(CFDictionaryRef dict, CFStringRef key) {
  CFTypeRef value = CFDictionaryGetValue(dict, key);
  CFT value_specific = CFCast<CFT>(value);
  if (value && !value_specific) {
    DLOG(WARNING) << GetValueFromDictionaryErrorMessage(key, value);
  }
  return value_specific;
}

You lose the ability to have the error message show the expected type name, but
I think that’s fine. You can also go to a small bit of extra trouble to
templatize a function that can provide the type name.

+
+GET_VAL_FROM_DICT_DEFN(CFArray);
+GET_VAL_FROM_DICT_DEFN(CFBag);
+GET_VAL_FROM_DICT_DEFN(CFBoolean);
+GET_VAL_FROM_DICT_DEFN(CFData);
+GET_VAL_FROM_DICT_DEFN(CFDate);
+GET_VAL_FROM_DICT_DEFN(CFDictionary);
+GET_VAL_FROM_DICT_DEFN(CFNull);
+GET_VAL_FROM_DICT_DEFN(CFNumber);
+GET_VAL_FROM_DICT_DEFN(CFSet);
+GET_VAL_FROM_DICT_DEFN(CFString);
+
 }  // namespace mac
 }  // namespace base