while loop inside for loop

Question

I read this sample code in a book. I can't figure out why this part of the following sample code's function declaration is necessary:

while (i <= n)
    p[i++] = '\0'; // set rest of string to '\0'

Here is the whole code:

#include <iostream>

const int ArSize = 80;

char * left(const char * str, int n = 1);

int main()
{
    using namespace std;
    char sample[ArSize];

    cout << "Enter a string:\n";

    cin.get(sample,ArSize);

    char *ps = left(sample, 4);
    cout << ps << endl;

    delete [] ps; // free old string

    ps = left(sample);
    cout << ps << endl;

    delete [] ps; // free new string
    return 0;
}
// This function returns a pointer to a new string
// consisting of the first n characters in the str string.
char * left(const char * str, int n)
{
    if(n < 0)
        n = 0;

    char * p = new char[n+1];
    int i;

    for (i = 0; i < n && str[i]; i++)
        p[i] = str[i]; // copy characters

    while (i <= n)
        p[i++] = '\0'; // set rest of string to '\0'

    return p;
}

I ran the code after I erased it and there was no problem.

Answer 1

The loop is unnecessary. Null-terminated strings end at the first null byte. If more memory was allocated than the actual string needs, it does not matter what’s in those extra bytes. All non-broken C-string handling code stops at the first null terminator. All that’s required is a single

p[i] = '\0';

after the for loop. However, that one null byte is mandatory. C-string functions depend on it and will happily overrun the allocated memory if it’s missing. Essentially they’ll (try to) keep going until they stumble upon the next null byte in memory. If that is past the allocated memory it causes undefined behaviour, resulting in a crash if you’re lucky; or corrupted data if you’re less lucky.

That said: Throw away that book yesterday. The code is a catastrophe from the first to the last line. It barely qualifies as C++. Most of it is plain C. And even as C code it’s highly questionable.

• Why to avoid using namespace std. @vol7ron pointed out in the comments that the major complaint is against using namespace std in headers. Here it’s used inside a function in a .cpp file, which lessens the impact significantly. Although in my opinion it is still worth avoiding. If you don’t know the implementation of your standard library in depth, you don’t really have an idea about all the symbols you pull into your scope. If you need it for readability, pulling in specific symbols (e.g. using std::cout;) is a better choice. Also, I’m confident I’m not alone in kind of expecting the std:: prefix. For example, std::string is what I expect to see. string looks slightly off. There’s always a lingering doubt that it might not be the std library string, but a custom string type. So, including the prefix can benefit readability as well.
• Why all the C-string pain? We’ve had std::string for a while now …
• Copying characters in a loop? Seriously? That’s what std::strcpy() is for.
• Raw new and delete everywhere: error prone because you have to keep track of the new/delete pairs manually to avoid memory leaks.
• Even worse: asymmetric owning raw pointers. left() allocates and returns a pointer; and it’s the caller’s responsibility to delete it. It doesn’t get more error prone than that.

… And these are only the problems that stick out on first glance.

What that piece of code should look like:

#include <iostream>
#include <string>

std::string left(const std::string& str, std::size_t len = 1);

int main()
{
    // getline can fail. If that happens we get an empty string.
    std::string sample;
    std::getline(std::cin, sample);

    auto ps = left(sample, 4);
    std::cout << ps << '\n';

    ps = left(sample);
    std::cout << ps << '\n';

    return 0;
}

// `len` may be longer than the string. In that case a copy
// of the complete input string is returned.
std::string left(const std::string& str, std::size_t len)
{
    return str.substr(0, len);
}