0

(I am a beginner to C++. But I am familiar with some other prog. languages, specially with Java.)

Can anyone please help me to find the flaw in this C++ code?

string & getFullName(string name, bool male){
    string fullName = name;
    if (male) {
        fullName = string(” Mr. ”) + fullName;
        return fullName;
    }
}
1
  • 11
    You're returning a reference to a local. You don't wanna do that.
    – Mysticial
    Jul 12, 2012 at 4:48

5 Answers 5

10

There are at least three problems:

  • First, you're returning a reference to the local variable.

  • Second, there is no return when male is false. I mean, what if the if block didn't get executed? What will your function return? There is no return statement for this case.

  • Third, “Mr. ” is not a string literal. "Mr. " is a string literal. Note the difference.

The fix can be this:

string  getFullName(string name, bool male)
{
    if (male) 
    {
        return string("Mr. ") + name;
    }
    else
    {
        return "Ms. " + name;  //No need to write even `string("Ms.")`
    }
}

No need of local variable.

Or simply:

string  getFullName(string name, bool male)
{
    return (male ? string("Mr. ") : string("Ms. "))  + name;
}
9
  • @Nawaz: Now you've brought up the endless style argument over whether the else belongs since the code above it always returns. Jul 12, 2012 at 4:56
  • 1
    Great answer. @Kasnel That "Stack" thing. Go study about it and expand ;-). en.wikipedia.org/wiki/Call_stack
    – Haix64
    Jul 12, 2012 at 7:19
  • 2
    @DavidSchwartz Would you explain a lil bit more please?
    – Haix64
    Jul 12, 2012 at 7:20
  • 1
    @Nawaz: I mean it serves no purpose in the execution of the code. Certainly those who argue it should be there believe it serves a purpose. And not that I disagree with you, but the obvious counter-argument would be that it reduces readability because when you see it, your first thought would be that its purpose was to affect the flow of the code, since that's an else's normal purpose -- you have to figure out that it doesn't do anything to the flow. Jul 12, 2012 at 7:36
  • 1
    @DavidSchwartz: In your example you are right, but in the example of the topic starter, there is no return after the if statement, and that is the problem that Nawaz was pointing at. Jul 12, 2012 at 7:37
2

You are returning a reference to a local variable. It ideally means fullName will die once the function getFullName returns, and if you try to assign it something like

string myName = getFullName("Adam",true);

myName will be garbage.

1

Don't return a reference to local and remove the single left curly:

void makeFullName(string &name, bool male){
    if (male) /* remove the curly */
        name = string(“Mr. ”) + name;
}
0
1

The scope of the fullname is limited to this function only, hence once this function ends or goes out of scope, fullname is removed from the stack. Therefore when you return the reference of fullname, you return the address of flushed up memory hence you will get garbage value. The other problem is when the condition of male is false, you are not returning anything.

0

You probably want something more like this:

std::string getFullName(string const &name, bool male) { 
    std::string ret = "Mr. ";
    if (male)
        return ret+name;
    return name;
}

Another possibility would also make it easier to add titles to females' names if you chose:

std::string getFullName(string const &name, bool male) { 
    static const std::string titles[] = {std::string(""), std::string("Mr. ")};

    return titles[male] + name;
}

If you do want a title added to women's names, just insert it in place of the empty string above.

4
  • tnx, but I need to tell you that I am a very beginner to C++. So actually I can't understand the code you've provided perfectly. What is this std::string means? & what is title[]? Is it an array?
    – Kadiam
    Jul 12, 2012 at 5:10
  • 1
    @Kasnel: std::string is the standard C++ string type. It's what your string should refer to, but I've written out the full name. Yes, titles is an array. Jul 12, 2012 at 5:14
  • So is it wrong if I write just as string instead of std::string?
    – Kadiam
    Jul 12, 2012 at 5:24
  • 2
    @Kasnel: You can make that work by putting using std::string; (or using namespace std; before your code. The latter is generally considered at least somewhat objectionable though (more accurately, quite a few people hate it intensely). Jul 12, 2012 at 5:27

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.