C-String problem

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

Code:

---

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

---

The code:

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:

Quote:

---

Originally Posted by syzn

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

Code:

---

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

---

The code:

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];

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

Code:

---

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

---

I get an error when building it saying:

Code:

---

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

---

This compiles for me.

Code:

---

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

---

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.

Quote:

---

Originally Posted by 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.

---

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:

Code:

---

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

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:

Code:

---

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!

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?

Quote:

---

Originally Posted by 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).

---

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.:ehh:

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

Code:

---

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

Code:

---

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

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

---

Quote:

---

Originally Posted by 2kaud

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

Code:

---

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.,

Code:

---

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

---

Quote:

---

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.

Quote:

---

Originally Posted by 2kaud

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

Quote:

---

Originally Posted by 2kaud

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.

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.

Quote:

---

Originally Posted by 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.

---

Take your own code and look at it carefully:

Code:

---

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:

Code:

---

#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:

Code:

---

#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

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.

Quote:

---

Originally Posted by 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.

---

How are you learning c++? You may like to look at these sites for more info
http://www.learncpp.com/
http://www.cplusplus.com/doc/tutorial/

I have a book called C++ Early Objects. I'll look into those links as well though, thank you.

Quote:

---

Originally Posted by syzn

I have a book called C++ Early Objects. I'll look into those links as well though, thank you.

---

Is that book the one by Gaddis, Walters & Muganda?

Yes

Quote:

---

Originally Posted by 2kaud

Code:

---

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

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

---

---

In addition to laserlight's post #10, I don't see any advantage in using the ternary operator here instead of a plain if-else. The result of the operation is discarded anyway.

Quote:

---

Originally Posted by Eri523

I don't see any advantage in using the ternary operator here instead of a plain if-else. The result of the operation is discarded anyway.

---

Guilty as charged:blush: In mitigation I was trying to identify a polymorphism bug at the time and I originally wrote

Code:

---

//Don't try this at home
std::cout << (last > str) ? *(last - 1) : "null";

---

but this doesn't compile of course! (incompatible operand types) So I just quickly changed it to

Code:

---

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

---

Quote:

---

Originally Posted by 2kaud

Code:

---

//Don't try this at home
std::cout << (last > str) ? *(last - 1) : "null";

---

---

:lol: In fact I considered suggesting that very variation to make the ternary operator at least half-way useful here, but came to the conclusion that it wouldn't compile before finishing my post. :)