tags:

views:

195

answers:

6

I have a simple function that takes a char and return a string, in C it looks like;

char* get_string(char c) {
  switch(c) {
    case 'A': return "some string";
    Case 'B': return "some other string";
...

And it works fine, but then I wanted my code to work in C++, and the C++ compilers throws a gazillions "deprecated conversion from string constant to ‘char*’". I understand the warning, but I'm not 100% sure what is the best way to implement the function so it will work, fast, on both C and C++. This function is being called a lot, it's an important bottleneck, so it has to be fast. My best try is;

char* get_string(char c) {
  char* str = (char*)malloc(50);
  switch(c) {
    case 'A':
      sprintf(str, "some string");
      return str;
    Case 'B':
      sprintf(str, "some other string");
      return str;
...
+14  A: 

Return a const char* instead of char*. The const in this context means "this pointer points to a constant pointee" - in other words, the caller cannot modify the returned string. This allows the compiler to place the strings in some memory location so the get_string() function can just return addresses to those strings. If the caller needs to modify the returned string, they can allocate their own buffer and copy it (preferably via std::string).

const char* get_string(char c) { 
  switch(c) { 
    case 'A': return "some string"; 
    case 'B': return "some other string"; 
...
In silico
Thanks, exactly what I wanted.
Paul Kern
@Paul, make sure to "accept" his answer so we know this question is closed.
Gunslinger47
exactly, the second example above makes the caller responsible to release the malloc'd memory.
gbrandt
+1  A: 

Make your original function return const char * instead char *. The problem is string literals are const. New compilers do not allow direct casting them to non-const type.

Stephen Chu
String literals *aren't* of type `const char[]` in C, no matter how new the compiler is (though they have always been unmodifiable).
caf
A: 

If you're returning a constant string (which in this case you are), your return value should be const char*. If you need to return a mutable string, the three ways to do it are, in general:

Force the caller to allocate the string beforehand

int get_string (char c, char* outstr, size_t outstr_len) 
{
   // Write the output into outstr using strcpy or similar
   // and return an error code if outstr_len is smaller than
   // the space required to store the string
}

This is the most idiomatic in C, but requires the most work on the caller's part.

Allocate the string on the caller's behalf

char* get_string (char c) 
{ 
   char* str = malloc(/* however big */);
   // ...
   return str;
}

Note that this method is relatively slow, and you will need to document that it is the caller's responsibility to free the returned string, and that it should be freed with free (as opposed to delete[]). You could use new[] and delete[] instead of malloc and free here, but I assume the only reason that you would do this is to inter-operate with C code.

Allocate a static buffer

char* get_string (char c) 
{ 
   static char* str[/* however big */];
   // ...
   return str;
}

Here, you must document how large the returned string is guaranteed to be, and that the caller is not to free it. The major drawback here is that subsequent calls to get_string will clobber previously returned strings.

Ed: Hm, markdown does not seem to like intermixing code and lists

Tyler McHenry
A: 

Use const char as the others say. However, if this function is truly a bottleneck, you should use a statically allocated array instead of a switch statement. You can improve the response time from O(n) to O(1) if you can simply make:

const char* MyStrings[100];

... initialize MyStrings ...

const char* get_string(char c) {
    return MyStrings[c-'A'];
}
Scott Stafford
although much less safe...
Anders K.
@Anders, it's only unsafe if you don't make it part of the function contract. It may be that the input to this function is only ever gathered from a data input that's already validated (such as a drop-down list containing A and B alone). In that case, it's perfectly acceptable to throw away all error checking as long as the API specifies that it only works with valid arguments.
paxdiablo
when doing a function it is often quite good to avoid doing assumptions about the input. if a function has (char c) as argument it implies (for a user of the function) that any char can be passed (when one day that function is used elsewhere - thus my comment about unsafe.
Anders K.
It's still O(1) if you check the range of the 'c' argument.
Arafangion
A: 
const char* get_string(char c) {
  static const char * case_a = "some string";
  static const char * case_b = "some other string";
  switch(c) {
    case 'A': return case_a;
    Case 'B': return case_b;
...
Those string literals are /already/ a 'const char *'. No need to bind them to a variable name, and no need to mandate that they're static, which may well already be the case, too.
Arafangion
@Arafangion, actually they're not const chars in C. C99 states that the type of a string literal is char, not const char (6.4.5/5). The only reason string literals are special is because attempts to modify them are undefined (6.4.5/6) but that doesn't make them const char. You're right however that the binding is unnecessary, by virtue of the fact they're converted to const by being returned (due to the function return type). They _are_ const chars in C++. which is probably what you were referring to.
paxdiablo
@paxdiablo: Interesting, I didn't realise that, but that said, isn't C a weakly typed language - if so, then does this matter? (Or does C99 try to do strong typing as an additional 'warning'?)
Arafangion
+1  A: 

You can combine some of the solutions already given to make it even faster but at some point you will hit diminishing returns, where the extra effort may not be worth the benefit gained.

I would both return a const char * using fixed strings and do it from a static array assuming you can guarantee that the source range is contigous (such as 'A' through 'C').

The use of a static constant character array means that there' minimal cost where a writable copy is not needed. The cost of creating this array will most likely be during compile time rather than run-time. If the caller actually wants a writable copy, it's up to them to do it with something like:

char *x = strdup ( get_string ('A'));

(and, yes, I know strdup isn't ISO-standard but it's trivial to write anyway). Also for raw speed, you can drop error checking and make the assumption that callers will not pass invalid values part of the contract.

So something like:

const char * get_string (char c) {
    static const char *arr[] = {
        "some string",              // A
        "some other string"         // B
    };                              // any other value is undefined behaviour.
    return arr[c-'A'];
}

And this is important, do not assume that this (or any other solution) will be fastest based on what you think you know. The optimisation mantra is "Measure, don't guess!" (and measure in a production-like environment).

paxdiablo