Trying to write a simple string reversal function in C++

Code junkies hangout here

Moderators: ChrisThornett, LXF moderators

Trying to write a simple string reversal function in C++

Postby stuarte9 » Sat Aug 20, 2011 5:25 pm

Hi,

As the header says, I'm trying to write a simple(!) string reversal function in C++.

First , here is the class:-

Code: Select all
class string
{
   public:                // universal access
      void assign(const char *st) { strcpy(s, st); len = strlen(st); }
      int  length() { return (len); }
      void print() { std::cout << s << "\nLength: " << len << "\n"; }
      void reverse();     // Reverses the contents of hidden data member s.
      friend string operator+(const string& a, const string& b);
   private:                 // restricted access to member functions
      char s[max_len];    // implementation by char(acter) array
      int  len;
};



Second, here is the relevant part of main() ;-

Code: Select all
// Next two lines for debug only.
   std::cout << "Checking the contents of string one." << std::endl;
   one.print();
   
   std::cout << std::endl << std::endl;
   one.reverse();       // Reverse contents of string
   one.print();

Finally, here is the definition for member function reverse():-

Code: Select all
void string::reverse()
{
   char temp[len];
   int j = len;
   
        // Work forwards from beginning of temp.
        // Work backwards from end of s.
   for (int i = 0; i <= len; i++, j--)
      temp[i] = s[j];

// Next two lines for debug only.
   std::cout << "\nChecking the contents of array temp.\n";
   std::cout << temp;
   
   j = len;
   for (int i = 0; i <= j; i++)
      s[i] = temp[i];
}


The code compiles without any errors or warnings. However, when I execute the program I get the following output (partial listing only, the rest of the program works just fine):-

Checking the contents of string one.
My name is Alan Turing.
Length: 23



Checking the contents of array temp.

Length: 16

As nothing is printed for array temp I'm assuming that it has not received anything from hidden data member s. (Both are array of char and should be assignment compatible.) Is my understanding correct ? As reverse() is a member function it should have access to private data member "s". Am I overlooking something obvious here ?

I'm wanting to do this in this manner rather than use a library function as I'm just getting started.

Any help that can be given with this would be very much appreciated.

Thanks in advance.

Stuart
stuarte9
 
Posts: 60
Joined: Mon Mar 08, 2010 5:03 pm
Location: Scotland

Postby pajo_ojap » Mon Aug 22, 2011 12:02 am

I'm kinda surprised no one has responded to this yet. But here goes. The main issue is the dreaded array index vs array size. In member reverse you set local variable 'j' to 'len' which is the length of the character array stored in your string object. You then use 'j' as the index to take from the back end of the stored string and save it into the front end of the local 'temp'. Array indexes start at 0, so for a string length of 16, you want your indexing to run from 0 to 15 ( or in this case from 15 to 0). What you are doing is starting at the null terminator of the string and working to one less than the front of the string (from 16 to 1) so you would never capture the first character. This is also why nothing was being printed as the first element of your reversed character array is the null terminator. This then brings up the next issue you would run into where the character string would not be properly null terminated for printing purposes (unless you wrote your own print to use the objects 'len' member).

So you need to set
Code: Select all
int j = len - 1
in member function reverse and then you have to set
Code: Select all
temp[i] = '\0';
after the for loop that reverses the array.

I compiled the code (with changes made for the missing parts to get it to compile) and it worked fine for me.


Good luck with it.
pajo_ojap
 
Posts: 2
Joined: Sun Aug 21, 2011 3:39 pm

Postby pajo_ojap » Mon Aug 22, 2011 2:44 pm

A thought popped into my head and I realized there was a different approach with using the null terminator. Since the string that its going into is already null terminated and the second loop structure
Code: Select all
j = len;
for (int i = 0; i <= j; i++)
s[i] = temp[i];
is set up to run from 0 to string length, you will need the line
Code: Select all
temp[i] = '\0';
as stated originally. but if you modify the second loop to only run from 0 to strength length - 1 you wouldn't need to add the null termination because it already exists in variable memeber 's' at the correct spot.

Hoping this is helpful
pajo_ojap
 
Posts: 2
Joined: Sun Aug 21, 2011 3:39 pm

Postby larcky » Sun Sep 11, 2011 8:33 pm

Hi
It's a shame you're not using real C++ strings (as you say you want to learn it the hard way) because then it's easy:
Code: Select all
#include <string>
#include <iostream>
#include <algorithm>

int main()
{
    std::string s1( "Here is a string..." );
    std::string s2( s1 );

    std::copy( s1.rbegin(), s1.rend(), s2.begin() );
    std::cout << "s2=" << s2 << "\n";

    return 0;

} // main


For C-style strings I think I'd rather use 'do...while' than a 'for' loop. Just seems a bit easier to read when you've got two indices flying about:
Code: Select all
#include <iostream>
#include <cstring>

int main()
{
    const char* s1 = "Here is a string...";
    char s2[ strlen(s1) + 1 ];

    // Be sure to start at final '.' rather than null.
    int from = strlen(s1) - 1;
    int to   = 0;

    do
    {
        s2[to] = s1[from];
        ++to;
        --from;
    } while ( from >= 0 );
    s2[to] = '\0';

    std::cout << "s2=" << s2 << "\n";

    return 0;

} // main


Regards, larcky
larcky
 
Posts: 19
Joined: Sun Nov 21, 2010 6:28 pm
Location: England


Return to Programming

Who is online

Users browsing this forum: Yahoo [Bot] and 0 guests

cron