🔒 Locked

Strange String Manipulation problem

Started by hammerstein_02 Sep 30, 2002 at 10:27 AM 14 replies 500+ views

Original Post

Sep 30, 2002 at 10:27 AM

Ok.. well I am just trying to assist a friend with a program that reverses a sentence. I wrote the program and my test data was to enter ''Terry is a moron'' , this string returns moron a is noromTerry So.. I thought my program wasn''t behaving quite correctly.. so I tried with several sets of data including Timmy is a moron, result: moron a is noromTimmy Bob is a moron, result: moron a is Bob I can''t work out why the strange behaviour, several other sentences worked fine.. why when 5 letters are used for the first word does it add the reversed moron on the end?

  
#include <iostream>
#include <stdlib.h>
#include <string.h>

char * reversestring(char*);

int main(int argc, char *argv[])
{
 char *sentence = new char[255];
 char *newsentence = new char[255];
 char *wrd;
 char *token = " ";

 cout << "Enter a sentence: ";
 cin.get(sentence,255);

 sentence = reversestring(sentence);
 
 cout << "Reversed Sentence: " << sentence << "\n";
  
 wrd = strtok(sentence,token);
 
 wrd = reversestring(wrd);
 strcat(newsentence,wrd); //First word

 
 while((wrd = strtok(NULL,token)))
 {
  wrd = reversestring(wrd);
  
  strcat(newsentence," "); //Need a space

  strcat(newsentence,wrd); //Add the word!

 }
 
 cout << "New Sentence: " << newsentence << "\n";
 
 system("PAUSE");
 
 delete newsentence;
 delete sentence;
}

//Reverse a given string character by character

char * reversestring(char *strn)
{
 char *mystr = new char[strlen(strn)];
 char *cur;
 int  idx = strlen(strn) - 1;
 int  id2 = 0;
 
 do
 {
  cur = &strn[idx];
  mystr[id2++] = cur[0];
  idx--;
 }while(idx > -1);
 
 return mystr;
}

Basiror

Sep 30, 2002 at 10:34 AM

    
for(int i=strlen(reversetring)-1,int a=0;i>=0,int i=strlen(reversetring);i--,a++)
{
newstring[a]=reversestring[i];
}

try something like this

[edited by - Basiror on September 30, 2002 11:35:42 AM]

[edited by - Basiror on September 30, 2002 11:36:21 AM]

Kurioes

Sep 30, 2002 at 10:44 AM

That would reverse the characters in a sentence.

hammerstein_02 is trying to reverse the word order I think.

-Thork-

Sep 30, 2002 at 10:49 AM

First thought: that code doesn''t work. Period. It stops functioning after pressing enter on the cin.get()

Other thoughts:
Your not deleting your strings properly:
delete sentence;
should be
delete [] sentence;
for example.

This may not be your problem, but I do notice that in your reversestring function, you are returning the address of a locally allocated pointer which is likely destroyed at the end of the function. That and the fact that the string that it returns a pointer to is NOT null terminated.

I would do something like this:

  
void reversestring(char *strn, char* out)
{
	int count=strlen(strn) - 1;

	for (int i=count; i>=0; i--) {
		out[i] = strn[count-i];
	}

	out[count + 1] = ''\0'';

	return;
}

So that the reversed string is returned through the out pointer which points do an array that already exists in the main function (newsentence for example)

Burning_Ice

Sep 30, 2002 at 10:50 AM

Hi

your problem lies in that your reversestring function you make the string one character too short and dont initialize it to 0. This has the effect that there is no ''\0'' quitting the string.

just change your code to this:

  
char * reversestring(char *strn)
{
 char *mystr = new char[strlen(strn)+1];
 memset ( mystr, 0, strlen(strn)+1);
 char *cur;
 int  idx = strlen(strn)-1;
 int  id2 = 0;
 
 do
 {
  cur = &strn[idx];
  mystr[id2++] = cur[0];
  idx--;
 }while(idx > -1);
 
 return mystr;
}

although i think your loop is still overly complicated

this would look nicer:

  
char * reversestring(char *strn)
{
	int len = strlen(strn);
	char *mystr = new char[len+1];
	memset ( mystr, 0, len+1);
	for ( int i = len-1; i >= 0; i-- )
		mystr[len-i-1] = strn[i];
 	return mystr;
}

Runicsoft -- home of my open source Function Parser and more

hammerstein_02

Sep 30, 2002 at 10:51 AM

Thank-you for the reply man.. yeah that was it. Put the code together really without much thought. (Am at work and occupying myself with this stuff!)

I don't do much string manipulation and overall am still a learner at this stage. Thanks for the lessons tho, they are greatly appreciated.

and the code did work before, just where I had five characters as my first word it misbehaved! I wrote it in DevC++ today and it compiled and worked fine the copy I pasted.

[edited by - hammerstein_02 on September 30, 2002 11:55:30 AM]

[edited by - hammerstein_02 on September 30, 2002 11:57:30 AM]

Burning_Ice

Sep 30, 2002 at 10:56 AM

quote:
Original post by -Thork-

This may not be your problem, but I do notice that in your reversestring function, you are returning the address of a locally allocated pointer which is likely destroyed at the end of the function.

well its the purpose of the new operator to allocate memory that does NOT get destroyed when leaving the scope, but only when using the delete operator on it, so this is no problem, although not the best coding practice since you can easily forget to delete [] a string that was new''d in another function.

Runicsoft -- home of my open source Function Parser and more

Cyberdrek

Sep 30, 2002 at 10:57 AM

quote:
Original post by Burning_Ice
your problem lies in that your reversestring function you make the string one character too short and dont initialize it to 0. This has the effect that there is no ''\0'' quitting the string.

Unless that''s a new behavior that I have never heard of but Zeroing out a string won''t necessarily put a ''\0'' at the end of the string. If you want a ''\0'' you have to put it yourself.

[Cyberdrek | the last true sorcerer | Spirit Mage - mutedfaith.com]

Burning_Ice

Sep 30, 2002 at 11:02 AM

quote:
Original post by Cyberdrek

Unless that''s a new behavior that I have never heard of but Zeroing out a string won''t necessarily put a ''\0'' at the end of the string. If you want a ''\0'' you have to put it yourself.

well if i have e.g. a char[10] string, memset all of its bytes to 0 (and ''\0'' is a zero byte!) and then read in less then 10 characters the char after the last read char IS 0 so the string functions know how to quit there.

Runicsoft -- home of my open source Function Parser and more

hammerstein_02

Sep 30, 2002 at 11:18 AM

Yeah.. I know my coding practice isn''t to hot on memory allocation right now, I will give that some time and experience hopefully. However, can you answer me this, why did it work for a lot of strings but not for any I had 5 letters in? And it was consistent with these..

civguy

Sep 30, 2002 at 11:19 AM

Slowness alert:

    
#include <iostream>
#include <string>
#include <boost/tokenizer.hpp>

std::string reverseWordOrder(const std::string& sentence) {
  boost::tokenizer<> tok(sentence);

  //create the result string that has words in reversed order

  std::string result;
  for(boost::tokenizer<>::iterator beg=tok.begin(); beg!=tok.end(); ++beg){
    result = *beg + " " + result;
  }

  if (result.length() == 0)
    return "";
  else
    //take care of the trailing space

    return result.erase(result.length()-1, result.length());
}

int main(){
  std::cout << reverseWordOrder("Terry is a moron") << std::endl;
  return 0;
}

...but it works (yeah, I'm just too lazy to debug your code)

[edited by - civguy on September 30, 2002 12:20:30 PM]

Burning_Ice

Sep 30, 2002 at 11:48 AM

quote:
Original post by hammerstein_02
However, can you answer me this, why did it work for a lot of strings but not for any I had 5 letters in? And it was consistent with these..

i''d say its something to do with your compiler...
when i ran your original code on VC++ 5 then, according to what sentence i try, it crashed, gave me a debug assertion error or just printed 255 characters of garbage

Runicsoft -- home of my open source Function Parser and more

civguy

Sep 30, 2002 at 1:33 PM

There''s one more bug that wasn''t addressed here yet (I think). You need to put
newsentence[0] = ''\0'';
somewhere in there before you use newsentence. Otherwise it may contain garbage (instead of an empty string) which causes a crash at the first strcat.

DrPizza

Sep 30, 2002 at 1:56 PM

Inplace.

  
#include <iostream>
template <typename T>
void reverse(T* start, T* end) // assume end is one past the end of the characters you want to swap

{
	for(;start < --end; ++start)
	{
		T temp = *start;
		*start = *end;
		*end = temp;
	}
}

template <typename T>
void reverseWords(T* start, T* end) // assume end is one past the end of the characters you want to swap

{
	reverse(start, end);
	T* walker = start;
	while(walker < end)
	{
		while(*walker != T('' '') && walker != end) ++walker;
		reverse(start, walker);
		start = ++walker;
	}
}



int main(int argc, char* argv[])
{
	using namespace std;
	char input[] = "This is my sentence";
	cout << input << endl;
	reverseWords(input, input + sizeof(input) - 1);
	cout << input << endl;
}

Stoffel

Sep 30, 2002 at 2:50 PM

Okie dokie, folks, let me introduce you to "string.h":

    
#include <iostream>

#include <string.h>

using namespace std;

int main ()
{
   char str[4096];
   cin.getline (str, 4096);
   char *pWord;
   while (NULL != (pWord = strrchr (str, ' ')))
   {
      cout << pWord+1 << ' ';
      *pWord = 0;
   }
   cout << str << endl;

   return 0;
}

[edited by - Stoffel on September 30, 2002 3:51:00 PM]

DrPizza

Sep 30, 2002 at 6:52 PM

That doesn''t actually reverse a sentence, it just prints it out in reverse order. You''d have to do that into a string or stringstream, tidy up the spaces, then write them back to the array in order to do what''s asked.

IMO.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.