C-String problem

[syzn]

Can someone help me figure out what I am doing wrong here? I keep getting this error after I input:

Unhandled exception at 0x54AE350B (msvcp110d.dll) in Random.exe: 0xC0000005: Access violation writing location 0x0131CA21.

The code:

#include <iostream>
#include <cstring>

//Prototypes
void lastChar(char *);

int main()
{
	const int LENGTH = 21;
	char *input = "";

	std::cin.getline(input, LENGTH);
	lastChar(input);

	return 0;
}

void lastChar(char *str)
{
	for(unsigned int count = 0; count < strlen(str); count++)
	{
		if(str[count] == '\0')
		{
			std::cout << str[count-1];
		}
		else
		{
			str++;
		}
	}
}

It builds without any errors. I am just trying to output the last character in the C-string. Am I doing this all wrong? D:

[GCDEF]

Can someone help me figure out what I am doing wrong here? I keep getting this error after I input:

Unhandled exception at 0x54AE350B (msvcp110d.dll) in Random.exe: 0xC0000005: Access violation writing location 0x0131CA21.

The code:

#include <iostream>
#include <cstring>

//Prototypes
void lastChar(char *);

int main()
{
	const int LENGTH = 21;
	char *input = "";

	std::cin.getline(input, LENGTH);
	lastChar(input);

	return 0;
}

void lastChar(char *str)
{
	for(unsigned int count = 0; count < strlen(str); count++)
	{
		if(str[count] == '\0')
		{
			std::cout << str[count-1];
		}
		else
		{
			str++;
		}
	}
}

It builds without any errors. I am just trying to output the last character in the C-string. Am I doing this all wrong? D:

input is a string literal. You can't write to it. Even if you could, you didn't allocate any memory for it other than the null terminator. Try

char input[LENGTH];

[syzn]

If I do that, it won't let me do this:

std::cin.getline(input, LENGTH);

I get an error when building it saying:

no instance of overloaded function "std::basic_istream<_Elem, _Traits>::getline [with _Elem=char, _Traits=std::char_traits<char>]" matches the argument list

[GCDEF]

This compiles for me.

int main()
{
	const int LENGTH = 21;
	char input[LENGTH];

	std::cin.getline(input, LENGTH);
	lastChar(input);

	return 0;
}

void lastChar(char *str)
{
	for(unsigned int count = 0; count < strlen(str); count++)
	{
		if(str[count] == '\0')
		{
			std::cout << str[count-1];
		}
		else
		{
			str++;
		}
	}
}

[syzn]

oops, I had left out a semicolon before it and compiler didn't pick it up. I got it to compile. Now the only problem left is that it doesn't do what I want it to do. :C

After you input, it doesn't output anything.

[Paul McKenzie]

oops, I had left out a semicolon before it and compiler didn't pick it up. I got it to compile. Now the only problem left is that it doesn't do what I want it to do. :C

After you input, it doesn't output anything.

What debugging have you done? Writing programs isn't just about getting C++ syntax to compile. You have to learn how to debug your own programs. Debugging programs is part and parcel of learning how to write a program.

If you did that, you would immediately see that this is no good:

if(str[count] == '\0')

How could str[count] ever be equal to 0 in that loop? The string is terminated with a NULL, so you will never encounter it if you loop from the beginning to the end of the string. But again, simple debugging using the debugger that comes with your compiler would have seen the problem.

Regards,

Paul McKenzie

[syzn]

I knew that line was the problem; I just didn't know the string terminates with a NULL. Even though everything I know dictates I should have known. Anyways, I got it to work with:

void lastChar(char *str)
{
	unsigned int count = 0;

	while(count <= strlen(str))
	{
		if(count == strlen(str))
			std::cout << str[count-1];
		else
			str++;

		count++;
	}
}

Thanks for your help!

[laserlight]

That is horribly inefficient: you call strlen on each iteration of the loop, and then you keep incrementing count just so that you can print str[count-1] when count == strlen(str). Why not just store the result of strlen(str) in count, then print str[count-1] if count is positive?

[2kaud]

That is horribly inefficient: you call strlen on each iteration of the loop, and then you keep incrementing count just so that you can print str[count-1] when count == strlen(str).

Its worse than that! count is always incremented by 1 but if the if test fails, then str is also incremented by 1! So everytime round the loop, the length of the c-string pointed to by str gets smaller and count gets bigger.

If you just want to output the last char of the c-string, try this

void  lastChar(char const* str)
{
char const *const last = strchr(str, '\0');

	(last > str) ? std::cout << *(last - 1) : std::cout << "null";
}

To check if str is NULL as per Paul's point in post #14, this could be coded as

void  lastChar(char const* str)
{
char const *const last = (str != NULL) ? strchr(str, '\0') : NULL;

	(last > str) ? std::cout << *(last - 1) : std::cout << "null";
}

[laserlight]

If you just want to output the last char of the c-string, try this

void  lastChar(char const* str)
{
char const *const last = strchr(str, '\0');

	(last > str) ? std::cout << *(last - 1) : std::cout << "null";
}

That looks like it will work, but I don't see the advantage of using strchr over strlen here, e.g.,

void printLastChar(char const* str)
{
    std::size_t count = strlen(str);
    (count > 0) ? std::cout << str[count - 1] : std::cout << "null";
}

[2kaud]

That looks like it will work, but I don't see the advantage of using strchr over strlen here, e.g.,

I prefer to use pointer reference rather than array access as tests using my compiler with my compiler options on my system have indicated that pointer reference is slightly quicker than using array access. In other respects they perform the same function. Personal preference.

[laserlight]

I prefer to use pointer reference rather than array access as tests using my compiler with my compiler options on my system have indicated that pointer reference is slightly quicker than using array access. (...) Personal preference.

Can't dispute that though I am surprised that the difference would be significant assuming you're using conventional patterns of array access and pointer [b]de[b]reference

In other respects they perform the same function.

Indeed, but there may be a difference in readability due to the abstraction called "string length", e.g., I think that strlen(str) is more descriptive than strchr(str, '\0'), though the latter could be more flexible say if the '\0' argument was changed to be a second parameter of the function. Consequently, I saw no reason to change from what syzn was already using when I made my suggestion in post #8.

[syzn]

woah, thanks for all the info. I don't know some of the code both of you posted, but I'll figure it out. I haven't learned much yet, hence, the horrible code I made.

[Paul McKenzie]

woah, thanks for all the info. I don't know some of the code both of you posted, but I'll figure it out.

Take your own code and look at it carefully:

void lastChar(char *str)
{
    unsigned int count = 0;
    while(count <= strlen(str))

Now, the string doesn't change, so why would the length of the string change? The more efficient thing to do is compute the length of the string one time, and store it in a variable:

#include <cstring>

void lastChar(char *str)
{
   unsigned int count = 0;
   size_t sLen = strlen( str );
   while( count <= sLen )
   {
      if (count == sLen)
          std::cout << str[count-1];
      else
          str++;
       count++;
   }
}

But why do this? You know the length of the string, so to get the last character is simple:

#include <cstring>

void lastChar(char *str)
{
   size_t sLen = strlen( str );
   if ( sLen > 0 )
       cout <<  str[sLen-1];
   else
       cout << "The string is empty";
}

No loop is needed. Note the check to ensure that str has characters.

There is still one flaw, and that is there is no check if str is a NULL pointer. You should check for str being NULL, otherwise calling strlen() on a NULL pointer could lead to bad things happening in your program. If it is NULL, then I guess you would consider 0 as the length and return immediately.

Regards,

Paul McKenzie

[syzn]

Thank you so much! Just a question if I may: How are "unsigned int" and "size_t" different? I looked it up but didn't really understand the explanation.