Beginner C++ Question

Question

I have followed the code example here

toupper c++ example

And implemented it in my own code as follows

void CharString::MakeUpper()
{
char* str[strlen(m_pString)];
int i=0;
str[strlen(m_pString)]=m_pString;
char* c;
while (str[i])
  {
    c=str[i];
    putchar (toupper(c));
    i++;
  }
}

But this gives me the following compiler error

CharString.cpp: In member function 'void CharString::MakeUpper()':
CharString.cpp:276: error: invalid conversion from 'char*' to 'int'
CharString.cpp:276: error:   initializing argument 1of 'int toupper(int)'
CharString.cpp: In member function 'void CharString::MakeLower()':

This is line 276

putchar (toupper(c));

I understand that toupper is looking for int as a parameter and returns an int also, is that the problem? If so how does the example work?

Answer 1

There is not a built-in conversion from char * to int, which is why the error occurs. Since you're trying to capitalize a character, you need to dereference the pointer.

putchar(toupper(*c));

Answer 2

You need to feed toupper() an int (or a char) instead of a char *, which is how you've declared c.

try:

char c;

Also,

char* str[strlen(m_pString)];

is an an array of pointers to characters, not just a single string.

This line:

str[strlen(m_pString)]=m_pString;

is an assignment to a bad pointer then, since there was no allocation.

Answer 3

Also,

char* str[strlen(m_pString)];
int i=0;
str[strlen(m_pString)]=m_pString;

is not valid C++ - arrays must be dimensioned using compile time constants - this is a C99 feature. And I really don't think the code would do what you want it to, even if it were legal, as you seem to be accessing one past the end of the array. It would be handy if you posted the complete class definition.

Answer 4

I don't think your code does what you want it to do and in fact if it compiled it would explode.

char* str[strlen(m_pString)]; // you've made an array of X C strings where 
                              // X is the length of your original string.
int i=0;


str[strlen(m_pString)]=m_pString; // You've attempted to assign the C string in your array
                                  // at location X to point at you m_pString.  X is the
                                  // same X as before and so is 1 past the end of the array
                                  // This is a buffer overrun.

I think what you actually wanted to do was to copy the content of m_pString into str. You'd do that like so:

char * str = new char[strlen(m_pString)];
memcpy(str, m_pString); // I may have the operands reversed, see the docs.

The easier way to do this though is to stop using C strings and to use C++ strings:

std::string str = m_pString;

There are more issues, but this should get you steer you more toward the right direction.

Answer 5

I'm going to go with the assumption that m_pString is a C style string (char *). You're doing way more fiddling than you need to be doing.

void CharString::MakeUpper()
{
   char* str = m_pString; // Since you're not modifying the string, there's no need to make a local copy, just get a pointer to the existing string.
   while (*str) // You can use the string pointer as an iterator over the individual chars
   {
      putchar (toupper(*str)); // Dereference the pointer to get each char.
      str++;   // Move to the next char (you can merge this into the previous line if so desired, but there's no need.
   }
}

In the example you cite, the reason it works is because of how the variables are declared.

int main ()
{
  int i=0;
  char str[]="Test String.\n";  // This is a compile time string literal, so it's ok to initialize the array with it.  Also, it's an array of `char`s not `char*`s.
  char c;  // Note that this is also a `char`, not a `char *`
  while (str[i])
  {
    c=str[i];
    putchar (toupper(c));
    i++;
  }
  return 0;
}

Because of the error-prone ways of using C strings, your best bet is std::string:

void CharString::MakeUpper()
{
   string str(m_pString);
   transform(str.begin(), str.end(), ostream_iterator<char>(cout), &toupper);
}