Remove wchar_t versions of StringPrintf from Android

base/stringprintf.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_STRINGPRINTF_H_
 #define BASE_STRINGPRINTF_H_
 
 #include <stdarg.h>   // va_list
 
 #include <string>
 
 #include "base/base_export.h"
 #include "base/compiler_specific.h"
 
 namespace base {
 
 // Return a C++ string given printf-like input.
 BASE_EXPORT std::string StringPrintf(const char* format, ...)
     PRINTF_FORMAT(1, 2);
-BASE_EXPORT std::wstring StringPrintf(const wchar_t* format, ...)
-    WPRINTF_FORMAT(1, 2);
-
 // Return a C++ string given vprintf-like input.
 BASE_EXPORT std::string StringPrintV(const char* format, va_list ap)
     PRINTF_FORMAT(1, 0);
 
 // Store result into a supplied string and return it.
 BASE_EXPORT const std::string& SStringPrintf(std::string* dst,
                                              const char* format, ...)
     PRINTF_FORMAT(2, 3);
+
+// Append result to a supplied string.
+BASE_EXPORT void StringAppendF(std::string* dst, const char* format, ...)
+    PRINTF_FORMAT(2, 3);
+
+// Lower-level routine that takes a va_list and appends to a specified
+// string.  All other routines are just convenience wrappers around it.
+BASE_EXPORT void StringAppendV(std::string* dst, const char* format, va_list ap)
+    PRINTF_FORMAT(2, 0);
+
+// wchar_t variants of above functions. Not supported by Android's libc.
+#if !defined(OS_ANDROID)

[jar (doing other things), 2012/07/24 18:15:34]

IMO, this grouping (by whether we're Android or not) to reduce the number of #if
statements, makes readability worse.

I don't like #if stuff (so I can sympathize with the motivation), but with
header files, folks expect to find similar functionality (e.g., StringPrintf()
for various flavors of strings) to be near either other.  I wouldn't expect to
have to look in a special non-android section to find a function :-/.

As a result, I'd rather leave the functions mostly where they naturally landed,
and put in the separate sets of #if statements around them.

[Iain Merrick, 2012/07/25 10:57:23]

Sure, no problem! I considered both and made the opposite call, but I'm happy to
defer to your decision. :)

I also considered adding a special macro -- HAVE_WPRINTF or some such. Let me
know if you think that's a good idea.

[jar (doing other things), 2012/07/25 18:54:04]

Mixed feelings:

Macros are frowned upon.  If you wanted to add one, you'd have to be REAL
careful to avoid conflicting with others, and plausibly with conventions(?).

Your suggestion would make this header more readable.

IMO, risks of problems via name pollution outweigh the gain.

I'd suggest adding one comment, before your first use of the ifdef here, saying
that: 

"OS_ANDROID does not have wchar_t, so several overloads are omitted"


On 2012/07/25 10:57:23, Iain Merrick wrote:

[Iain Merrick, 2012/07/26 11:15:56]

Done.

+BASE_EXPORT std::wstring StringPrintf(const wchar_t* format, ...)
+    WPRINTF_FORMAT(1, 2);
+
 BASE_EXPORT const std::wstring& SStringPrintf(std::wstring* dst,
                                               const wchar_t* format, ...)
     WPRINTF_FORMAT(2, 3);
 
-// Append result to a supplied string.
-BASE_EXPORT void StringAppendF(std::string* dst, const char* format, ...)

[jar (doing other things), 2012/07/24 18:15:34]

This is interesting.... it was previously only advertised for non-android, and
now you're going to advertise and presumably use it.

Where are each of these used in this CL that required making them visible?

...and if you want them visible... I'd rather use separate ifdefs around the
sibling items (like line 40-41) that you want to hid.

[Iain Merrick, 2012/07/25 10:57:23]

Hmm, probably more interesting than I intended! I'll clean up with separate
ifdefs.

-    PRINTF_FORMAT(2, 3);
 // TODO(evanm): this is only used in a few places in the code;
 // replace with string16 version.
 BASE_EXPORT void StringAppendF(std::wstring* dst, const wchar_t* format, ...)
     WPRINTF_FORMAT(2, 3);
 
-// Lower-level routine that takes a va_list and appends to a specified
-// string.  All other routines are just convenience wrappers around it.
-BASE_EXPORT void StringAppendV(std::string* dst, const char* format, va_list ap)
-    PRINTF_FORMAT(2, 0);
 BASE_EXPORT void StringAppendV(std::wstring* dst,
                                const wchar_t* format, va_list ap)
     WPRINTF_FORMAT(2, 0);
+#endif
 
 }  // namespace base
 
 // Don't require the namespace for legacy code. New code should use "base::" or
 // have its own using decl.
 //
 // TODO(brettw) remove these when calling code is converted.
 using base::StringPrintf;
 
 #endif  // BASE_STRINGPRINTF_H_