Thread: WTF con't

Wow, kinda boggled by my first C++ program:

Code:

#include <iostream>
#include <string>
using namespace std;

class nlist : public string {
	public:
		nlist(string in) {
			assign(in);
		}
		int extract(size_t *pos) {
			int retv = 0, m = 1;
			size_t end, place;
			while (at(*pos) < '0'|| at(*pos) > '9') {
//				printf("%d %d\n",*pos,at(*pos)); fflush(stdout);
				(*pos)++;
			}
			end = *pos;
			while (at(end) > '0'&& at(end) < '9') end++;
			place = end;
			end--;
			while (end>=*pos) {
				if (end<0) break;
	printf("pos %d end %d\n",*pos, end); fflush(stdout);
				retv += (at(end--)-48)*m;
				m*=10;
			}
			*pos = place;
			return retv;
		}
};

int main() {
	string src="1,44,56,33\n";
	nlist nums(src);
	size_t len = nums.size(), pos = 0;

	cout << nums;
	while (pos<len) cout << nums.extract(&pos);

	return 0;
}

Considering the conditions in red, how is it possible that I get this at runtime:

1,44,56,33
pos 0 end 0
pos 0 end -1
terminate called after throwing an instance of 'std:ut_of_range'
what(): basic_string::at
Aborted

"If (end<0) break;" then it proceeds to the next line and reports end = -1.
???????????????

Maybe the problem lies here, considering that there is no bounds checking other than the use of at():

Code:

while (at(*pos) < '0'|| at(*pos) > '9') {
    (*pos)++;
}

Again, please stop publicly inheriting from std::string. I know that inheritance is a novelty for you right now, but it is better that you do not start developing bad habits.

Your first C++ program? Really? With the comparison statements you've made before?

First of all, size_t is unsigned, hence the name (negative size?).

Second what laserlight said, don't inherit std::string (or most of the standard implementation besides streams and such for that matter).

Originally Posted by laserlight

Maybe the problem lies here, considering that there is no bounds checking other than the use of at():

Nope. It's because end is -1:

Code:

			while (end>=*pos) {
				if (end<0) break;
	printf("pos %d end %d\n",*pos, end); fflush(stdout);    

Again, the output:
pos 0 end 0
pos 0 end -1
I do not understand how a loop which has that condition and that break would continue iterating. It should not be possible for that printf to execute while end < 0.

Okay, I guessing it's because size_t is an unsigned type....

Vis, deriving from std::string, yes, I understand this not a wise design move, I'm not planning on using it for anything except getting a feel for the syntax.

Originally Posted by MK27

Okay, I guessing it's because size_t is an unsigned type....

Yep. Man, this C++ is weird.

Originally Posted by MK27

Man, this C++ is weird.

size_t is inherited from C.

This is how I might implement the algorithm you have in mind with the kind of usage that you have in mind:

Code:

#include <iostream>
#include <string>

/* Extracts next integer in str starting from pos and assigns it to result.
 * Returns the position of the first unprocessed character or std::string::npos
 * if there there are no more integers left to process.
 */
std::string::size_type
extract(const std::string& str, std::string::size_type pos, int& result);

int main()
{
    std::string::size_type pos = 0;
    int num;
    while ((pos = extract("1,44,56,33\n", pos, num)) != std::string::npos)
    {
        std::cout << num << '\n';
    }
    std::cout.flush();
}

std::string::size_type
extract(const std::string& str, std::string::size_type pos, int& result)
{
    const std::string::size_type size = str.size();

    // Skip non-digit characters.
    for (; pos < size && (str[pos] < '0' || str[pos] > '9'); ++pos)
    {
        // Nothing to do here.
    }

    // Find last character of integer to be extracted.
    std::string::size_type end_pos = pos;
    for (; end_pos < size && (str[end_pos] >= '0' && str[end_pos] <= '9'); ++end_pos)
    {
        // Nothing to do here.
    }

    if (pos < end_pos)
    {
        // Extract integer portion into an int.
        result = 0;
        int multiplier = 1;
        for (std::string::size_type i = end_pos; i > pos;)
        {
            result += multiplier * (str[--i] - '0');
            multiplier *= 10;
        }
        return end_pos;
    }
    else
    {
        // Integer not found.
        return std::string::npos;
    }
}

That said, I realised that if we do not follow your algorithm so exactly, we can simplify, e.g.,

Code:

std::string::size_type
extract(const std::string& str, std::string::size_type pos, int& result)
{
    static const char* const digits = "0123456789";

    pos = str.find_first_of(digits, pos);

    if (pos == std::string::npos)
    {
        return std::string::npos; // Integer not found.
    }

    // Find one past the last character of non-negative integer to be extracted.
    std::string::size_type end_pos = str.find_first_not_of(digits, pos);
    if (end_pos == std::string::npos)
    {
        end_pos = str.size();
    }

    // Extract non-negative integer value into an int.
    result = 0;
    int multiplier = 1;
    for (std::string::size_type i = end_pos; i > pos;)
    {
        result += multiplier * (str[--i] - '0');
        multiplier *= 10;
    }
    return end_pos;
}

I guess I have to get on reading that book. Quick question(s):

1) so std::string::size_type is implementation specific, or always size_t?

Anyway thanks for that. I actually didn't scroll down far enough to notice stuff like "find_first_not_of". std::string, pretty neat.

Originally Posted by MK27

1) so std::string::size_type is implementation specific, or always size_t?

std::string is a typedef for std::basic_string<char>, which has a default third template argument of std::allocator<char>. Now, the size_type member for std::basic_string is a typedef for the size_type member of this allocator. Since std::allocator<char>::size_type is a typedef for std::size_t, std::string::size_type is thus an alias for std::size_t.

Originally Posted by MK27

I actually didn't scroll down far enough to notice stuff like "find_first_not_of". std::string, pretty neat.

Yeah, though it is a little less efficient than checking than comparing with '0' and '9'. A generic algorithm with a predicate could be used to get around that, but then writing a predicate currently often requires more boilerplate than is desirable, at least without the use of say Boost.Lamdba and such.