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!

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

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.

Originally Posted by abk

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

yeah, right. It is giving a better output after that correction, but I am sure it has some more bugs

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.