Am I using C++ pointers and references correctly?

Question

I am a newbie to C++ programming and am currently reading this book called Jumping to C++ by Alex Allain. I have finished the pointers chapter and I am doing the exercises at the end of the chapter.

The following paragraphs are the exercises and included are my own solutions:

Write a function that prompts the user to enter his or her first name and last name, as two separate values. This function should return both values to the caller via additional pointer (or reference) parameters that are passed to the function. Try doing this first with pointers and then with references. (Hint: the function signature will look be similar to the swap function from earlier!)

// pointersExercise01.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "iostream"
#include "string"

using namespace std;

void printName(string *pointFirstName, string *pointLastName){

    cout << endl << "Hello " << *pointFirstName << " " << *pointLastName;
}

void printName(string &pointFirstName, string &pointLastName){
    cout << endl << "Hi " << pointFirstName << " " << pointLastName;
}


int _tmain(int argc, _TCHAR* argv[])
{
    string firstName, lastName;

    cout << "Enter First Name ";
    cin >> firstName;
    cout << endl << "Enter Last Name ";
    cin >> lastName;

    /*FUNCTION POINTERS*/
    printName(&firstName, &lastName); // print out using pointers

    /*REFERENCE POINTERS*/
    printName(firstName, lastName);

    system("pause");
    return 0;
}

The next exercise is to modify the program I wrote for exercise 1 so that instead of always prompting the user for a last name, it does so only if the caller passes in a NULL pointer for the last name.

// nullPointerExercise3.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "iostream"
#include "string"

using namespace std;

void userNames(string *pointerFirstName, string *pointerLastName){

    string firstN = *pointerFirstName;
    string LastN = *pointerLastName;
    bool flag;

    do{
        if(LastN == "NULL" ){
            cout << "Again what is your last name? ";
            cin >> LastN;

            flag = true; 
        }else{
            flag = false;
        }

    }while(flag != false);
}

int _tmain(int argc, _TCHAR* argv[])
{
    string firstName, lastName;

    cout << "FIRSTNAME: ";
    cin >> firstName;
    cout << "\n" << "LASTNAME: ";
    cin >> lastName;

    userNames(&firstName, &lastName);

    system("pause");
    return 0;
}

If possible, I'd like to know if I did right for both of the exercise and if I am in the right direction. Comments and tips will be helpful.

Answer 1

I'm not sure whether this should be reviewed yet or not. I can't decide what critiques are things that you haven't learned yet but will, and things that you haven't learned yet and might not. (Also, there's a handful of errors that should have been found with basic testing.)

Anyway here's a few things that jumped out at me:

• There's no notion of const correctness at all in your code
• system("pause") is bad -- it's slow, and implementation dependent
  • if you want to see the output, run it in a shell directly instead of using system("pause") as a hack
• When pointerLastName is NULL, string LastN = *pointerLastName; is undefined behavior and will probably crash.
• if(LastN == "NULL" ) makes no sense
  • a NULL pointer being dereferences does not turn into the string "NULL" -- it crashes (typically)
• Your do {} while() loop in the second program is broken
• using namespace std; pollutes the global namespace. It's typically better to just qualify it directly like std::string or import specific things using std::string;.
  • In small, simple programs, it tends to not matter. It can become a bad habit though.
  • #include "iostream" should be #include <iostream> -- I'm actually a bit surprised that compiles.

Also, for what it's worth, I wouldn't use pointers in either of these two programs. References are a better choice. Obviously this is a learning exercise though, so this critique doesn't really apply.

---

Based on our comments back and forth, there seems to be some confusion surrounding pointers. I'm not sure if it's me misunderstanding what your saying, or a misunderstanding of pointers, but either way, I figure I'll elaborate a bit.

if(LastN == "NULL" )

That is checking if LastN is equal to the string "NULL". That is not the same thing as checking for a NULL pointer:

string* pNullStr = NULL;
string nullStr = *pNullStr; //undefined behavior (almost certainly a crash)

The only way a string is going to have a value of NULL is if it is actually set to NULL:

string ns = "NULL";
if (ns == "NULL") {
    //this will happen
}

In other words, your userNames function isn't prompting the user for a last name only "if the caller passes in a NULL pointer for the last name" (as your instructions say). What it's doing is prompting for a last name only if the caller passes in a string (not a pointer to a string) equivalent to "NULL".

Based on your code, the only way a last name is going to be prompted for is if you type "NULL" in when prompted for a last name (in the main code).

Perhaps this was by design, but it directly contradicts your exercise instructions.

---

Also, while I'm at it with the editing, if you wanted to be really careful, you could check all of the input operations. In fact, really you should always check, but in a program this simple, it would be a bit of a pain.

To clarify:

cout << "FIRSTNAME: ";
cin >> firstName;

That can fail. What if the user writes EOF or if the user pipes in an empty file? That means the read is going to fail.

if (!(std::cin >> firstName)) {
    std::cerr << "Fatal error: reading last name failed";
    return EXIT_FAILURE; //(defined in cstdlib)
}

---

On a stylistic note, not sure why I didn't spot this before: if your loop in userNames is actually what you meant it to be, you can write it much more cleanly:

while (lastN == "NULL") {
    std::cout << "Again what is your last name? ";
    std::cin >> lastN; //Once again, this should be checked
}

Answer 2

You haven't done what the first exercise asked. It asks you to write a function that prompts the user for the name. You have written a function that prints the user's name. Effectively, you have done the opposite of what you were asked to do. As a result you've missed the point of the excersize.

The second one does what the excersize says, but the excersize did not ask you to keep asking. To simply follow the excersize you should not have a loop.