Thread: Word Sort--Again, but this time in C++

  1. #1
    Super unModrator
    Join Date
    Dec 2007
    Posts
    321

    Question Word Sort--Again, but this time in C++

    Hello,

    I have posted the same kind of program in C but it doesn't seem to work. So I decided to write it again from scratch, this time in C++, and it is working but is buggy.

    This code is supposed to take five names as input and then arrange them in alphabetical order.

    Earlier I was making it for 5 inputs but then restricted myself for just two,to make it less complicated, but on the same lines as I would for five.

    Sorry for the use of "non standard" things (like goto,C style strings) and bad indention (if any) I have used in there. I really don't know what are vectors, so please don't tell me to use them.

    Maybe there's an easy way to do the same but I want to stick with this method.

    Code:
    #include "stdafx.h" //MS Visual C++ 2008 Express
    
    
    #include<iostream>
    using namespace std;
    void swap(char*,char*);
    int main()
    {
    	char word[2][20],temp[20];
    	int x,y;
    	for(x=0;x<2;x++)
    	{
    		cout<<"\nEnter a word: ";
    		cin>>temp;
    		for(y=0;temp[y]!='\0';y++)
    			word[x][y]=temp[y];
    		word[x][y]='\0';
    	}
    	for(y=0;word[0][y]!='\0'&& word[1][y]!='\0';y++)
    	{
    		cout<<endl<<"Comparing "<<word[0][y]<<" and "<<word[1][y];
    		cout<<endl<<(int)word[0][y]<<"and"<<(int)word[1][y];
    		if((int)word[0][y]!=(int)word[1][y])
    		{
    
    			if((int)word[0][y]>(int)word[1][y])
    			{
    				cout<<endl<<"Control is here";
    				swap(&word[0][0],&word[1][0]);
    				break;            
    
    			}
    			else
    				goto display; //sorry for using goto.
    			
    		}
    	}
    
    display:
    	for(x=0;x<2;x++)
    	{
    		cout<<endl;
    		for(y=0;word[x][y]!='\0'&& word[x][y]!='\n';y++)
    			cout<<word[x][y];
    	}
    	cout<<endl;
    	return 0;
    }
    void swap(char*str1,char*str2)
    {
    	char temp;
    	int a=0,b=0; 
    	while(a!=1 || b!=1)
    	{
    		temp=*str1;
    		*str1=*str2;
    		*str2=temp;
    		if(a!=1)
    		{
    			str1++;
    			if(*str1=='\0')
    				a=1;
    		}
    		else
    			str1++;
    		if(b!=1)
    		{
    			str2++;
    			if(*str2=='\0')
    				b=1;
    		}
    		else
    			str2++;
    
    	}
    }
    It is taking the inputs, and giving the result output, but sometimes it gives a lot of extra special characters following it.

    Please suggest changes, and reasons for the problem I mentioned above.

    Thanks!

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    When you copy the strings in "swap" you are not copying the final zero-byte, just finding that it is zero and then saying "done" for that string.

    Also:
    Code:
    	if(a!=1)
    		{
    			str1++;
    			if(*str1=='\0')
    				a=1;
    		}
    		else
    			str1++;
    
    So, why not move the str1++ out of the if/else code and "just do it". Likewise of course for the second "same again" if statement.

    Edit: And where is this C++ - it is still using C-style strings, not C++ strings, which would make the job much easier, since you can compare a C++ std::string with plain "a > b" comparison, rather than individual letter comparison [of course, behind the scenes it still compares characters].

    --
    Mats
    Last edited by matsp; 02-04-2008 at 08:05 AM.
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Super unModrator
    Join Date
    Dec 2007
    Posts
    321
    When you copy the strings in "swap" you are not copying the final zero-byte, just finding that it is zero and then saying "done" for that string.
    Thanks! that's bug no.1........but maybe there are more.
    So, why not move the str1++ out of the if/else code and "just do it". Likewise of course for the second "same again" if statement.
    I don't want it to happen twice in case (a !=1) evaluates out to be true.

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by abk View Post
    Thanks! that's bug no.1........but maybe there are more.


    I don't want it to happen twice in case (a !=1) evaluates out to be true.
    Yes, so if it's outside of teh if/else [obviously in BOTH cases]:
    Code:
                    str1 ++;
    	        if(a!=1)
    		{
    			if(*str1=='\0')
    				a=1;
    		}
    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  5. #5
    Super unModrator
    Join Date
    Dec 2007
    Posts
    321
    yeah, right. It is giving a better output after that correction, but I am sure it has some more bugs

  6. #6
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Hey good stuff, your lexicographical compare will work now!
    One thing you can do to simplify it a bit though: Once you enter the if statement about two of the characters not being equal, then after checking if it is less-than or greater-then, you can simply break - always. That way you can remove the goto (which incidentally was foing the same job as a break anyway), and move the break out of that if-statement and up one level into the other if-statement.

    Notice how your swap function is getting quite huge now? That's largely because you're swapping one character at a time. You could simplify it by declaring a temporary string of sufficient size, you can do this:
    Code:
    strcpy(temp, str1);
    strcpy(str1, str2);
    strcpy(str2, temp);
    Just 3 lines.
    Of course the next step is to do it using real C++ where you no longer use 'char', but instead use std::string.
    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. Replies: 16
    Last Post: 11-23-2007, 01:48 PM
  2. brace-enclosed error
    By jdc18 in forum C++ Programming
    Replies: 53
    Last Post: 05-03-2007, 05:49 PM
  3. need help in time zone
    By Gong in forum C++ Programming
    Replies: 2
    Last Post: 01-03-2007, 04:44 AM
  4. Killing someones grandparents
    By nickname_changed in forum A Brief History of Cprogramming.com
    Replies: 37
    Last Post: 09-07-2003, 07:56 AM
  5. merge sort and selection sort and time spent on both
    By misswaleleia in forum C Programming
    Replies: 3
    Last Post: 06-04-2003, 02:24 PM