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.

The loop isn't needed, but the p[i++] = '\0' is needed if you want to treat p as a null-terminated byte string. — Some programmer dude
– Some programmer dude, Commented May 26, 2018 at 18:31
Also, please learn how to indent your code. It most likely was indented in the book, and while indentation is not needed by the compiler it do help people to read the code. — Some programmer dude
– Some programmer dude, Commented May 26, 2018 at 18:32
And the while loop isn’t nested inside the for loop - there is no opening brace immediately after the closing ) of the for statement so it only loops over p[i] = str[i]; That confusion/lack of clarity is the reason some coding standards require all for/while/if/else statements to be followed by { } around the statement(s) they control even if only one statement is involved it is much simpler to read. — DisappointedByUnaccountableMod
– DisappointedByUnaccountableMod, Commented May 26, 2018 at 18:33
You can replace it with p[i] = '\0';, one terminating null should be plenty — Killzone Kid
– Killzone Kid, Commented May 26, 2018 at 18:47
The while loop is ridiculous. All that is needed after the for loop is p[i] = 0; to properly terminate the string. The amount of memory allocated for p is also excessive if the length of the input string str is less than n. Which book is this? — catnip
– catnip, Commented May 26, 2018 at 18:49

besc · Accepted Answer · 2018-05-27 07:43:41Z

3

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);
}

edited May 27, 2018 at 7:43

answered May 26, 2018 at 20:07

besc

2,66715 silver badges11 bronze badges

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

3 Comments

vol7ron Over a year ago

No offense, but he set the namespace in main, which seems to be good enough for this example. While I like the mentioning to be cautious, especially if this is an older book, I don't think setting a namespace in a function is a poor decision. Yes, the risk of collisions is a reality, but the readability of the code is also important.

Georgy90 Over a year ago

The book is "C++ Primer Plus" 6th edition by Stephen Prata.

besc Over a year ago

@vol7ron Hm, maybe complaining about using namespace std has become a bit too automatic. I still think it’s worth avoiding. But this is a minor case and I should update the answer.

Collectives™ on Stack Overflow

while loop inside for loop

1 Answer 1

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related