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

base/mac/foundation_util.h

Index: base/mac/foundation_util.h
diff --git a/base/mac/foundation_util.h b/base/mac/foundation_util.h
index f7821de08cdb09bca58aaca59da82bd689407e29..bd5fe01cf3852d911f5b39b0f8d8ac524e2c534d 100644
--- a/base/mac/foundation_util.h
+++ b/base/mac/foundation_util.h
@@ -13,6 +13,8 @@
 
 #include "base/base_export.h"
 #include "base/logging.h"
+#include "base/mac/scoped_cftyperef.h"
+
 
 #if defined(__OBJC__)
 #import <Foundation/Foundation.h>
@@ -96,12 +98,23 @@ BASE_EXPORT FilePath GetUserLibraryPath();
 //   returns - path to the application bundle, or empty on error
 BASE_EXPORT FilePath GetAppBundlePath(const FilePath& exec_name);
 
+// Helper function for GetValueFromDictionary to create the error message
+// that appears when a type mismatch is encountered.
+std::string GetValueFromDictionaryErrorMessage(CFStringRef key,
+                                        CFStringRef expected_type_ref,

[Mark Mentovai, 2011/11/13 01:28:22]

Bad alignment on the indent.

+                                        CFTypeRef value);
+
 // Utility function to pull out a value from a dictionary, check its type, and
 // return it.  Returns NULL if the key is not present or of the wrong type.
 BASE_EXPORT CFTypeRef GetValueFromDictionary(CFDictionaryRef dict,
                                              CFStringRef key,
                                              CFTypeID expected_type);
 
+// Utility function to pull out a value from a dictionary, check its type, and
+// return it.  Returns NULL if the key is not present or of the wrong type.
+BASE_EXPORT template<class T>

[Mark Mentovai, 2011/11/13 01:28:22]

Remember what I said? typename sounds better than class here.

+T GetValueFromDictionary(CFDictionaryRef dict, CFStringRef key);

[Mark Mentovai, 2011/11/13 01:28:22]

Why did you declare this here? You can just have the declaration and definition
down below. You can also move the declaration of
GetvalueFromDictionaryErrorMessage down there, so that it’s near the caller that
it’s subservient to. There will be no other callers.

[KushalP, 2011/11/13 12:30:27]

Thought I'd killed this with the patch set.

+
 // Retain/release calls for memory management in C++.
 BASE_EXPORT void NSObjectRetain(void* obj);
 BASE_EXPORT void NSObjectRelease(void* obj);
@@ -218,10 +231,10 @@ namespace mac {
 //     base::mac::GetValueFromDictionary(some_dict,
 //                                       CFSTR("a_key"),
 //                                       CFStringGetTypeID()));
-BASE_EXPORT template<class T>
+BASE_EXPORT template<typename T>
 T CFCast(const CFTypeRef& cf_val);
 
-BASE_EXPORT template<class T>
+BASE_EXPORT template<typename T>
 T CFCastStrict(const CFTypeRef& cf_val);
 
 #if defined(__OBJC__)
@@ -248,7 +261,7 @@ T CFCastStrict(const CFTypeRef& cf_val);
 //
 // NSString* str = base::mac::ObjCCastStrict<NSString>(
 //     [ns_arr_of_ns_strs objectAtIndex:0]);
-BASE_EXPORT template<class T>
+BASE_EXPORT template<typename T>
 T* ObjCCast(id objc_val) {
   if ([objc_val isKindOfClass:[T class]]) {
     return reinterpret_cast<T*>(objc_val);
@@ -256,7 +269,7 @@ T* ObjCCast(id objc_val) {
   return nil;
 }
 
-BASE_EXPORT template<class T>
+BASE_EXPORT template<typename T>
 T* ObjCCastStrict(id objc_val) {
   T* rv = ObjCCast<T>(objc_val);
   DCHECK(objc_val == nil || rv);
@@ -265,6 +278,33 @@ T* ObjCCastStrict(id objc_val) {
 
 #endif  // defined(__OBJC__)
 
+// Utility function to pull out a value from a dictionary, check its type, and
+// return it.  Returns NULL if the key is not present or of the wrong type.
+// Is a cleaner implementation of base::mac::GetValueFromDictionary() above.
+BASE_EXPORT template<typename T>
+T GetValueFromDictionary(CFDictionaryRef dict, CFStringRef key) {
+  CFTypeRef value = CFDictionaryGetValue(dict, key);
+  T value_specific = CFCast<T>(value);
+
+  if (!value)

[Mark Mentovai, 2011/11/13 01:28:22]

Rid this entire function of early returns. Everything can just return
value_specific down below.

Making this change means getting rid of this conditional, changing the next one
to be (value && !value_specific), and getting rid of the |return NULL|.

+    // Can't return 'value' as it's a CFTypeRef. Provide a type T return.
+    return value_specific;
+
+  if (!value_specific) {
+    // TODO: Get the string value of the type name T.

[Mark Mentovai, 2011/11/13 01:28:22]

Can you think of a good way to do this?

[KushalP, 2011/11/13 12:30:27]

If I'm honest I don't know "what" I'm looking for. I played around with using
typeid(value_specific).name() but that just seems to stacktrace in interesting
ways.

Now that it's not a specialisation I don't think it should use CFGetTypeID
against value_specific.

I'll look a little deeper myself first and see how far I get.

[Mark Mentovai, 2011/11/14 14:12:05]

KushalP wrote:
You want a function that can be called like

std::string TypeNameForCFType(CFArrayRef cfo);
std::string TypeNameForCFType(CFDictionaryRef cfo);
std::string TypeNameForCFType(CFStringRef cfo);

And that returns "CFArray", "CFDictionary", and "CFString" in these cases. Does
that help put you on the right track?

+    std::string type_name = "INSERT TYPE NAME";
+    ScopedCFTypeRef<CFStringRef> expected_type_ref(CFStringCreateWithCString(
+        kCFAllocatorDefault,
+        type_name.c_str(),
+        kCFStringEncodingUTF8));
+    DLOG(WARNING) << GetValueFromDictionaryErrorMessage(key,
+                                                        expected_type_ref,
+                                                        value);
+    return NULL;
+  }
+  return value_specific;
+}
+
 }  // namespace mac
 }  // namespace base