Absolute Doozy of a problem I don't understand

This is a discussion on Absolute Doozy of a problem I don't understand within the C++ Programming forums, part of the General Programming Boards category; My goal is to take a string and do various modifications with it like reversing it, printing it, printing it ...

  1. #1
    Registered User Hexadakota's Avatar
    Join Date
    Jan 2008
    Posts
    21

    Absolute Doozy of a problem I don't understand

    My goal is to take a string and do various modifications with it like reversing it, printing it, printing it up to n places, and comparing it to another string using the class my_string.

    This is what I have so far

    Code:
    #include <iostream>
    #include <string>
    
    using namespace std;
    
    class my_string {
          public:
                 my_string() : len(0)
                             { s = new char[1];assert(s != 0); s[0] = 0; }
                 my_string(const my_string& str);       //copy constructor
                 my_string(const char* p);              //conversion constructor
                 ~my_string() { delete []s; }
                 int strcmp(const my_string& a) const;
                 void print(int n) const;               //int-overloaded print
                 void print() const { cout << s; }
                 void strrev() const;
          private:
                  char* s;
                  int len;
    };
    This is the class. There's the default constructor, the copy constructor and the conversion constructor, and the destructor. It's just supposed to take a string, a pointer to char. I don't think there's anything wrong with it (But I dunno).


    Code:
    void my_string::print(int n) const
    {
        if (n < 0 || n > len)
         cout << "error";
       else
        for (int i = 0; i < n; i++)
             cout << s[i];
         cout << endl;       
    }
    This function is for taking a string to be printed up to n places. I haven't had any problems with it.

    Code:
    int my_string::strcmp(const my_string& a) const
    {
        int i;
        for (i = 0; s[i] && a.s[i] && (s[i] != a.s[i]); ++i) 
            ;
        return (s[i] - a.s[i]);
    }
    This is my first problem. My goal is to send two strings to this function and it returns a 0 if the strings are lexicographically equal, a positive number if the first string (s) is greater, and a negative number if the second string (a.s) is greater. For some reason I'm just never getting the right return numbers. It will say two strings are equal when they're not, (It says Gladiator and Gladiolus are equal)

    Code:
    void my_string::strrev() const
    {
         int i;
         
         char* temp;
         temp = new char[len + 1];
         for(i = 0; i < (len); i++)
                 temp[i] = len - i;
         temp[i] = '\0';
         strcpy(s, temp);
         delete[] temp;
    }
    This is my next problem. My goal with this function is to reverse the string passed to my_string. For some reason, I keep getting garbage. I can't figure out why.

    Code:
    my_string::my_string(const char* p)
    {
        len = strlen(p);
        s = new char[len + 1];
        assert(s !=0);
        strcpy(s, p);
    }
    
    my_string::my_string(const my_string& str) : len(str.len)
    {
     s = new char[len + 1];
     assert(s != 0);
     strcpy(s, str.s);
    }
    Just the copy and conversion constructors.

    Code:
    int main()
    {
        my_string s1( "gladiator" );
        my_string s2( "gladiolus" );
        my_string s3( "xyz" );
        my_string s4( "abcd" );
        my_string s5( "same" );
        my_string s6( "same" );
        
        s1.print();                    //print the string as is
        
        cout << endl;
       
        int return_value;
        return_value = s1.strcmp(s2);          //
        
        if (return_value == 0){
        s1.print(); 
        cout << " is lexicographically equal to ";
        s2.print();
        cout << "\n" << endl;
        }
        else if (return_value < 0)
           cout << "String 1 is lexicographically less than String 2\n" << endl;
        else
           cout << "String 1 is lexicographically greater than String 2\n" << endl;
        
        s1.print(5);                   //print the string up to the Nth place
        cout << endl;
        s1.strrev();                   //reverse the string
        s1.print();
        
        system("pause");
      
    }
    Just the main function.

    I can't see what's wrong. I don't know if my syntax or if my logic is off. Can someone point out something I don't see?

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Don't you want your for-loop to run while the two strings are equal, not while they are inequal?

    Edit: Oh and
    Code:
    void my_string::strrev() const
    {
         int i;
         
         char* temp;
         temp = new char[len + 1];
         for(i = 0; i < (len); i++)
                 temp[i] = s[len - i];
         temp[i] = '\0';
         strcpy(s, temp);
         delete[] temp;
    }
    Although I didn't check to see whether you're including the \0 at the beginning or not.
    Last edited by tabstop; 03-11-2008 at 07:21 PM.

  3. #3
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    Well, for the strrev, you could simplify it and speed it up:

    Code:
    void my_string::strrev() const
    {
    	int i;
    	char c ;
    	if (len <= 1) return ; 
    	for ( i = 0; i < len/2 ; i++)
    		c = s[i] ; 
    		s[i] = s[len-1-i] ; 
    		s[len-1-i] = c ; 
    	} 
    }
    Todd
    Mac and Windows cross platform programmer. Ruby lover.

    Quote of the Day
    12/20: Mario F.:I never was, am not, and never will be, one to shut up in the face of something I think is fundamentally wrong.

    Amen brother!

  4. #4
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    strcmp: (s[i] != a.s[i]) should use ==. You want to go through the strings as long as they're equal.

    strrev:
    Code:
         for(i = 0; i < (len); i++)
                 temp[i] = len - i;
    You basically assign a descending integer sequence to the reversed string. You want to index into s with (len - i) (actually (len - i - 1)), not assign the value directly.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  5. #5
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    To answer your question, this is why strrev does not work
    Code:
      temp[i] = len - i;
    Ask yourself, what does "len" equal each loop?
    Mac and Windows cross platform programmer. Ruby lover.

    Quote of the Day
    12/20: Mario F.:I never was, am not, and never will be, one to shut up in the face of something I think is fundamentally wrong.

    Amen brother!

  6. #6
    Registered User Hexadakota's Avatar
    Join Date
    Jan 2008
    Posts
    21
    Ahh! You're all right about the strrev. I really was just assigning it a decrementing value that wasn't even related to the string. Duh...

    EDIT: You guys were right about strcmp too. Such a mindless mistake. Thanks for all the help!
    Last edited by Hexadakota; 03-11-2008 at 07:46 PM.

  7. #7
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,295
    This (corrected) for-loop can be simplified furthur:
    Code:
        for (i = 0; s[i] && a.s[i] && (s[i] == a.s[i]); ++i)
        ;
    If you take a look at the and-ed expressions you'll notice that the second one can be deduced from the first and the third:
    Code:
        for (i = 0; s[i] && (s[i] == a.s[i]); ++i)
        ;
    I.e. if s[i] is true and s[i] == a.s[i] is true then it follows that a.s[i] is true, so that test is redundant.
    Last edited by iMalc; 03-11-2008 at 11:48 PM.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Laptop Problem
    By Boomba in forum Tech Board
    Replies: 1
    Last Post: 03-07-2006, 05:24 PM
  2. tricky output problem, dont understand
    By panfilero in forum C Programming
    Replies: 6
    Last Post: 11-11-2005, 11:30 PM
  3. Replies: 5
    Last Post: 11-07-2005, 10:34 PM
  4. searching problem
    By DaMenge in forum C Programming
    Replies: 9
    Last Post: 09-12-2005, 01:04 AM
  5. half ADT (nested struct) problem...
    By CyC|OpS in forum C Programming
    Replies: 1
    Last Post: 10-26-2002, 08:37 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21