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

base/mac/foundation_util.h

 // Copyright (c) 2011 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.
 
 #ifndef BASE_MAC_FOUNDATION_UTIL_H_
 #define BASE_MAC_FOUNDATION_UTIL_H_
 #pragma once
 
 #include <CoreFoundation/CoreFoundation.h>
 
 #include <string>
 #include <vector>
 
 #include "base/base_export.h"
 #include "base/logging.h"
+#include "base/mac/scoped_cftyperef.h"
+
 
 #if defined(__OBJC__)
 #import <Foundation/Foundation.h>
 #else  // __OBJC__
 class NSBundle;
 class NSString;
 #endif  // __OBJC__
 
 class FilePath;
 
@@ skipping 63 matching lines @@
 // Returns the ~/Library directory.
 BASE_EXPORT FilePath GetUserLibraryPath();
 
 // Takes a path to an (executable) binary and tries to provide the path to an
 // application bundle containing it. It takes the outermost bundle that it can
 // find (so for "/Foo/Bar.app/.../Baz.app/..." it produces "/Foo/Bar.app").
 //   |exec_name| - path to the binary
 //   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);
 
 // CFTypeRefToNSObjectAutorelease transfers ownership of a Core Foundation
 // object (one derived from CFTypeRef) to the Foundation memory management
 // system.  In a traditional managed-memory environment, cf_object is
 // autoreleased and returned as an NSObject.  In a garbage-collected
 // environment, cf_object is marked as eligible for garbage collection.
 //
@@ skipping 96 matching lines @@
 // triggering any DCHECK.
 //
 // Example usage:
 // CFNumberRef some_number = base::mac::CFCast<CFNumberRef>(
 //     CFArrayGetValueAtIndex(array, index));
 //
 // CFStringRef some_string = base::mac::CFCastStrict<CFStringRef>(
 //     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__)
 
 // ObjCCast<>() and ObjCCastStrict<>() cast a basic id to a more
 // specific (NSObject-derived) type. The compatibility of the passed
 // object is found by checking if it's a kind of the requested type
 // identifier. If the supplied object is not compatible with the
 // requested return type, ObjCCast<>() returns nil and
 // ObjCCastStrict<>() will DCHECK. Providing a nil pointer to either
 // variant results in nil being returned without triggering any DCHECK.
 //
 // The strict variant is useful when retrieving a value from a
 // collection which only has values of a specific type, e.g. an
 // NSArray of NSStrings. The non-strict variant is useful when
 // retrieving values from data that you can't fully control. For
 // example, a plist read from disk may be beyond your exclusive
 // control, so you'd only want to check that the values you retrieve
 // from it are of the expected types, but not crash if they're not.
 //
 // Example usage:
 // NSString* version = base::mac::ObjCCast<NSString>(
 //     [bundle objectForInfoDictionaryKey:@"CFBundleShortVersionString"]);
 //
 // 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);
   }
   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);
   return rv;
 }
 
 #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
 
 // Stream operations for CFTypes. They can be used with NSTypes as well
 // by using the NSToCFCast methods above.
 // e.g. LOG(INFO) << base::mac::NSToCFCast(@"foo");
 // Operator << can not be overloaded for ObjectiveC types as the compiler
 // can not distinguish between overloads for id with overloads for void*.
 BASE_EXPORT extern std::ostream& operator<<(std::ostream& o,
                                             const CFErrorRef err);
 BASE_EXPORT extern std::ostream& operator<<(std::ostream& o,
                                             const CFStringRef str);
 
 #endif  // BASE_MAC_FOUNDATION_UTIL_H_