My function seems too simple to work..

This is a discussion on My function seems too simple to work.. within the C++ Programming forums, part of the General Programming Boards category; I wrote a function that compares two strings and it compiled fine but it seems too simple to work. I'm ...

  1. #1
    Refugee face_master's Avatar
    Join Date
    Aug 2001
    Posts
    2,052

    My function seems too simple to work..

    I wrote a function that compares two strings and it compiled fine but it seems too simple to work. I'm just afraid I overlooked something really big...
    Code:
    bool CompareStrings(char string1[], char string2[])
    {
    	int i = 0; // counter
    
    	while(i < sizeof(string1))
    	{
    		// Check character...
    		if(string1[i] != string2[i])
    		{
    			return FALSE;
    		}
    		
    		i++;
    	}
    	
    	return TRUE;
    }

  2. #2
    ....
    Join Date
    Aug 2001
    Location
    Groningen (NL)
    Posts
    2,386
    You always assume both strings have equal length, this assumption may not always be correct.

    You calculate the length of the string as sizeof(string1). Note that string1 is the address of string1[0]. If pointers on your system are 4 bytes, then only the first 4 characters will be compared.

    That means that "hello1" and "hello2" are equal when compared with your function.

  3. #3
    Registered User Mario's Avatar
    Join Date
    May 2002
    Posts
    317
    Adding to what Shiro already said, when you name an array simply by it's name, you are actually accessing it's memory location.

    So if you want to create a pointer to an array you write:
    Code:
    int* pmyarray = null; //declare and initialize array
    int myarray[20] = {0}; //declares and initializes my array. All 20 elements are 0.
    pmyarray = myarray; //assign myarray to the pointer
    Notice that I didn't use the & operator on the last line exactly because 'myarray' alone already points to a memory location. The memory location of myarray[0]

    Now to calculate your string lenght...
    My prefered method is exactly by using pointers to arrays. Since a string is an array of chars...

    Code:
    bool CompareStrings(char string1[], char string2[])
    {
        char* pstring1 = string1;
        char* pstring2 = string2;
        
        long lenstring1 = 0;     //for VERY large strings :P
        long lenstring2 = 0;
    
        while(*pstring1){                     
          pstring1++;}
        while(*pstring2){                     
          pstring2++;}
    
        lenstring1 = pstring1 - string1;
        lenstring2 = pstring2 - string2;
    
        if(lenstring1 != lenstring2) return FALSE;
    
        //... include here your changed while loop
    }
    This warrants an explanation.
    I start by declaring and initializing the pointers and the variables that will hold the strings lenght.
    The while loop is a bit harder to explain. Since the pointers created point to arrays, they behave a little different then other pointers. When you add 1 to a pointer, you actually are telling the pointer to point to the next element of the array. Consider this:

    pstring1 = string1 //point to element 1 of string1 (string1[0])
    pstring1 = pstring1 + 1 //point to element 2 of string1 (string1[1])
    pstring1 = pstring1 + 1 //point to element 3 of string1 (string1[2])

    So the the while loop tests *pstring1 and checks if it is a value (a character) or the NULL character ('\0') that finishes all strings. If it is. it will stop. If it is not, it increments the pointer (pstring1++ is about the same as pstring1 = pstring1 + 1) and tests it's new position.

    When the loop finishes, the pointer will be positioned at the last element of the array. Now you only need to get this last position memory address minus the first position (remember that an array name alone is actually the memory address of its first element) and you'll get the number of characters... aka, the size of the string in characters.

    The last line is just a shortcut to avoid processing the remaining of your code. if the strings don't have equal lenght they are obviously not equal.

    Just change the while loop on your own code to accomodate lenstring1 and add it after the return FALSE; statement above.
    Regards,
    Mario Figueiredo
    Using Borland C++ Builder 5

    Read the Tao of Programming
    This advise was brought to you by the Comitee for a Service Packless World

  4. #4
    Seeking motivation... endo's Avatar
    Join Date
    May 2002
    Posts
    537
    This made me curious, is there any problems with my version?

    Code:
    bool compStrings( char* a, char* b )
    {
         if( a.strlen( ) != b.strlen( ) )
         {
              return false;
         }
    
         for( int i = 0; i < a.strlen( ); i++ )
         {
              if( a[ i ] != b[ i ] )
              {
                   return false;
              }
         }
         return true;
    }

  5. #5
    Registered User Mario's Avatar
    Join Date
    May 2002
    Posts
    317
    I'm simply assuming he wants to make it from scratch. Otherwise he would include string.h and use strcmp to compare two strings

    [EDIT]
    Actually one could say there is something wrong with your function. Not by design, mind you. But because you are using string.h to compute strlen(), that same header has strcmp which compares 2 strings already. A bird's eye view of your project would then reveal you have 2 functions that do the exact same thing.
    [/EDIT]
    Last edited by Mario; 05-26-2002 at 07:25 AM.
    Regards,
    Mario Figueiredo
    Using Borland C++ Builder 5

    Read the Tao of Programming
    This advise was brought to you by the Comitee for a Service Packless World

  6. #6
    Registered User
    Join Date
    Apr 2002
    Posts
    362
    endo,

    char* a and char* b are not objects of the "string" class and, therefore, any reference to 'a.strlen()' and 'b.strlen()' is going to generate a compile-time error for you. (Double-check your use of the "dot operator". )

    This, however, will work:
    Code:
    bool compStrings( char* a, char* b )
    {
         if( strlen(a) != strlen(b) )
         {
              return false;
         }
    
         for( unsigned int i = 0; i < strlen(a); i++ )
         {
              if( a[ i ] != b[ i ] )
              {
                   return false;
              }
         }
         return true;
    }
    P.S. I agree with Mario. We seem stuck (if that's the word) with char arrays and char pointers when the 'string' class is so much more powerful. (I'm including myself here, but it does seem curious.)

    "When the only tool you own is a hammer, every problem begins to resemble a nail." Abraham Maslow

  7. #7
    Registered User Dual-Catfish's Avatar
    Join Date
    Sep 2001
    Posts
    802
    How about... this!

    Code:
    bool compStrings( char* a, char* b )
    {
    	if ((sizeof(a)/sizeof(a[0])) != (sizeof(b)/sizeof(b[0])))
    	{
    		return false;
    	}
    
    	for( int i = 0; i < (sizeof(a)/sizeof(a[0])); i++ )
    	{
    		if (a[i] != b[i])
    		{
    			return false;
    		}
    	}
     	return true;
    }

  8. #8
    Registered User Mario's Avatar
    Join Date
    May 2002
    Posts
    317
    Excellent!
    Regards,
    Mario Figueiredo
    Using Borland C++ Builder 5

    Read the Tao of Programming
    This advise was brought to you by the Comitee for a Service Packless World

  9. #9
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,670
    Sidenote:

    For extra benefit, temporarily erase the leading whitespace, since this would affect the length of the string because you would probably want "this" and " this" to compare equal....



    ITSA
    Socket Library!

  10. #10
    Registered User
    Join Date
    Apr 2002
    Posts
    362
    Something we need to be aware of is the comparison of constants.

    In D-C's example, my compiler (Borland 5.0) throws out a warning that the test always evaluates to "false" and that any code which should execute if "true" is "unreachable".

    Indeed, when I changed either "string", the function returned "true" to main(), regardless.

    This would seem to be a result of passing char pointers to the function, though, for the life of me, I don't know how D-C's test evaluates as a comparison of constants.
    Whenever the compiler encounters a constant comparison that (due to the nature of the value being compared) is always true or false, it issues this warning and evaluates the condition at compile time.

    For example:

    void proc(unsigned x){

    if (x >= 0) /* always 'true' */
    {
    ...
    }
    }
    I'm up for any insight here.

    "When the only tool you own is a hammer, every problem begins to resemble a nail." Abraham Maslow

  11. #11
    the hat of redundancy hat nvoigt's Avatar
    Join Date
    Aug 2001
    Location
    Hannover, Germany
    Posts
    3,139
    sizeof(a)/sizeof(a[0]))

    Thats sizeof(char*) / sizeof( char ) which is 4 / 1, which is a constant value, no matter what string you take.

    Always use const char*, after all, you don't want to change the strings you get, so you can as well communicate that to your user.

    Example:

    Code:
    bool StringCompare(const char * a, const char * b)
    {
    	while( (*a != 0) && (*a == *b) )
    	{
    		a++;
    		b++;
    	}
    	return ( *a == *b );
    }
    hth
    -nv

    She was so Blonde, she spent 20 minutes looking at the orange juice can because it said "Concentrate."

    When in doubt, read the FAQ.
    Then ask a smart question.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Seg Fault in Compare Function
    By tytelizgal in forum C Programming
    Replies: 1
    Last Post: 10-25-2008, 03:06 PM
  2. Screwy Linker Error - VC2005
    By Tonto in forum C++ Programming
    Replies: 5
    Last Post: 06-19-2007, 02:39 PM
  3. We Got _DEBUG Errors
    By Tonto in forum Windows Programming
    Replies: 5
    Last Post: 12-22-2006, 04:45 PM
  4. <Gulp>
    By kryptkat in forum Windows Programming
    Replies: 7
    Last Post: 01-14-2006, 12:03 PM
  5. Replies: 1
    Last Post: 04-02-2003, 06:50 PM

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