2

I was wondering, I normally use std::string for my code, but when you are passing a string in a parameter for a simply comparison, is it better to just use a literal?

Consider this function:

bool Message::hasTag(string tag)
{
    for(Uint tagIndex = 0; tagIndex < m_tags.size();tagIndex++)
    {
        if(m_tags[tagIndex] == tag)
            return 0;
    }

    return 1;
}

Despite the fact that the property it is making a comparison with is a vector, and whatever uses this function will probably pass strings to it, would it still be better to use a const char* to avoid creating a new string that will be used like a string literal anyway?

Improve this question
12
  • 6
    Parameters of class/struct type should always be passed by reference (probably const reference) unless you have a specific reason otherwise. Commented Jul 5, 2010 at 16:17
  • 1
    You should not be "using namespace std;" in your code. There will be trouble, I promise. Commented Jul 5, 2010 at 16:18
  • 3
    @amro using namespace std; is perfectly OK in source files, just not in headers. Commented Jul 5, 2010 at 16:20
  • 4
    @amro: You should only avoid importing any namespace (not just std) within header files. You're free to do whatever you want inside your .cpp files. So always say std::string in your header files, but your .cpp files often (usually?) are perfectly fine with a using namespace std;. Commented Jul 5, 2010 at 16:22
  • 2
    @Neil - yes, indeed there are cases when values will be faster than references. One should check with a profiler before deciding that one is better than the other for any given situation. Before checking with a profiler one is only doing guess based micro-optimizations. There are of course times when you can know that copying will be slow but in cases of small objects, like string, you really don't unless you've tested it. Commented Jul 6, 2010 at 6:16

3 Answers 3

Reset to default
16

If you want to use classes, the best approach here is a const reference:

bool Message::hasTag(const string& tag);

That way, redudant copying can be minimized and it's made clear that the method doesn't intend to modify the argument. I think a clever compiler can emit pretty good code for the case when this is called with a string literal.

Passing a character pointer requires you to use strcmp() to compare, since if you start comparing pointers directly using ==, there will be ... trouble.

Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

It will have to construct a string object, no matter how clever the compiler is.
Neil what do you mean by having to construct a string object? Doesn't the compiler just "exchange" the "&" with "*" and properly interpret any dereferencing syntax?
@ctinnist: If you're passing a reference (or a pointer) to a string object, you still have to have a string object at some point.
@Frerich: well, yes, otherwise you'll need to have a char* array somewhere... the OP is concerned about implicit copies when passing strings to functions, not the object creation itself.
@Frerich: of course there is an improvement, m_tags[tagIndex] == tag is easier to read (shorter) than strcmp(m_tags[tagIndex], tag) == 0. I'm not saying there's a performance improvement, I never tried to imply that all characters don't have to be inspected.
|
2

Short answer: it depends.

Long answer: std::string is highly useful because it provides a lot of utility functions for strings (searching for substrings, extracting substrings, concatenating strings etc.). It also manages the memory for you, so the ownership of the string cannot be confused.

In your case, you don't need either. You just need to know whether any of the objects in m_tags matches the given string. So for your case, writing the function using a const char *s is perfectly sufficient.

However, as a foot note: you almost always want to prefer std::string over (const) char * when talking about return values. That's because C strings have no ownership semantics at all, so a function returning a const char * needs to be documented very carefully, explaining who owns the pointed to memory (caller or callee) and, in case the callee gets it, how to free it (delete[], delete, free, something else).

Improve this answer

Comments

1

I think it would be enough to pass an reference rather than value of string. I mean:

bool Message::hasTag(const string& tag)

That would copy only the reference to the original string value. Which must be created somwhere anyway, but outside of the function. This function would not copy its parameter whatsoever.

Since m_tags is a vector of strings anyway (I suppose), const string& parameter would be better idea.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.