Programming Forums - Returning An Array Of Characters

Programming Forums (http://www.programmingforums.org/forumindex.php)

- C++ (http://www.programmingforums.org/forum15.html)

- - Returning An Array Of Characters (http://www.programmingforums.org/showthread.php?t=10733)

Returning An Array Of Characters

How can I return an array of characters from a function? I know there's always the option of returning a pointer, but when I try that, I'm probably doing it wrong because I have to use a for loop to reference the address of each individual pointer.

It'd be easiest to do it this way as well, so that I can just cout the function call, without needing to do anything to it.

#include <iostream>

using namespace std;





// CONSTANTS

const int MAX_SIZE = 256;





int get_size(char text[MAX_SIZE])

{

  for (int i=0;i<MAX_SIZE;i++)

  {

    if (!(text[i]))

      return i;

  }

  return MAX_SIZE;

}



int locate(char text[MAX_SIZE], char from[MAX_SIZE], 

           int size_f)

{

  int i2 = 0;

  

  for (int i1 = 0;i1 < MAX_SIZE;i1++)

  {

    if (text[i1] == from[i2])

    {

      i2 ++;

      if (i2 == size_f)

        return i1 - size_f + 1;

    }

    else if (!(text[i1]))

      return -1;

    else

      i2 = 0;

  } 

  

  return -1;

}



char* replace(char text[MAX_SIZE], char to[MAX_SIZE], 

              int where, int size_f, int size_t)

{

  char new_text[MAX_SIZE];

  

  int offs1 = size_f+where;

  int offs2 = size_t+where;

  

  // append text before change

  for (int i1=0; i1 <= where; i1++)

    new_text[i1] = text[i1];      



  // append change

  for (int i2=0; i2 <= size_t; i2++)

    new_text[i2+where] = to[i2];

  

  // append text after change

  for (int i3=0; i3 <= get_size(text)-size_f-where; i3++)

    new_text[i3+offs2] = text[i3+offs1];

  

  return new_text;

}



int main()

{

  char text[MAX_SIZE] = "This is some text, yaddi yaddi.";

  char from[MAX_SIZE] = "some";

  char to[MAX_SIZE]   = "no";

  

  int size_f = get_size(from);

  int size_t = get_size(to);

  

  int where = locate(text, from, size_f);

  

  if (where == -1)

    cout << "Value was not found anywhere inside the text";

  else  

    cout << replace(text, to, where, size_f, size_t);

    

  cin.sync();

  cin.get();

  return 0;   

}

>How can I return an array of characters from a function?
You can't. Either return a pointer, or wrap the array in an object so that it can be copied as a single entity.

>I'm probably doing it wrong
Yes. Yes you are.

int get_size(char text[MAX_SIZE])

{

  for (int i=0;i<MAX_SIZE;i++)

  {

    if (!(text[i]))

      return i;

  }

  return MAX_SIZE;

}

That's just funky. After the loop, i will be MAX_SIZE anyway, so why confuse readers more than you have to?

int get_size ( char text[] )

{

  int i = 0;



  while ( i < MAX_SIZE && text[i] != '\0' )

    ++i;



  return i;

}

Of course, even better would be taking the max size as an argument rather than hard coding some arbitrary macro that destroys reusability.

Tell me, what's wrong with the following code:

char* replace(...)

{

  char new_text[MAX_SIZE];

  

  ...

  

  return new_text;

}

Too subtle? You're returning a pointer to local data. new_text is destroyed when replace returns, so your pointer to it refers to indeterminate data. This is undefined behavior. You must either dynamically allocate your memory, use an object that has an appropriate copy constructor, pass the array as an argument, or declare the array as static and suffer the pitfalls of that particular option.

>cin.sync();
That's not guaranteed to do what you think.

By the way, you're commenting the wrong parts of your code. Anything that isn't immediately obvious should have a comment, or be changed so that it's immediately obvious. The one thing I wondered while reading your code is precisely the comments you should have added: why are you reinventing the wheel when the standard library supports a huge portion of what you're trying to do?

Quote:

Originally Posted by Sane

How can I return an array of characters from a function?

This question has been asked a few times. An un-surprisingly quick search yielded the following answer:
http://www.programmingforums.org/for...36&postcount=5

Narue and Cache have answered your question quite well. I'll just add a few little specific notes;

1) Your functions are essentially recreating the wheel of the C standard library. For example, your get_size() function achieves more or less the same thing as the strlen() function that is specified in <string.h>. Except that you are adding a few other quirks that aren't particularly useful.

2) Since this is a C++ group you might want to use the standard string class (std::string which is specified in the header <string>). This is (rather loosely) an object wrapper for an array of char. And can be passed to an output stream directly.

Quote:

Originally Posted by grumpy

Your functions are essentially recreating the wheel of the C standard library.

That was the intention of this exercise. Using an array of characters was also another part of the exercise.

Thank you for the help.

By the way, Narue, I'm not sure if you were calling my approach to it incorrect. Because all I had to do was add the word static before the declaration of "new_text". Which is still returning a pointer, but it's not as if I had to do much.

Quote:

Originally Posted by Sane

By the way, Narue, I'm not sure if you were calling my approach to it incorrect. Because all I had to do was add the word static before the declaration of "new_text". Which is still returning a pointer, but it's not as if I had to do much.

Narue was describing your approach as incorrect, and she's right. Your implication that you're actually right because fixing the problem isn't necessarily a lot of work (eg adding a static keyword) is crap. You don't have to do much to put wheel nuts on correctly when changing a flat tyre either. But the not putting those nuts in place or not tightening them correctly has major safety implications, and neglecting to do so is therefore plainly wrong. Imagine the response in a court of law, after someone has had a major accident, if your defence is "all I had to do was tighten the nuts".

>By the way, Narue, I'm not sure if you were calling my approach to it incorrect.
Your approach was broken, and therefore incorrect. I thought I made that abundantly clear.

>Because all I had to do was add the word static before the declaration of "new_text".
You'll notice that I went out of my way to mention that a static local variable has pitfalls. Unsurprisingly, you've chosen to pick the worst of your options (next to the undefined behavior you already had) and learn for yourself why it's a bad idea. That's really the best way to learn, in my opinion. :)

>Which is still returning a pointer, but it's not as if I had to do much.
In all honesty, I feel that your design sucks and needs to be scuttled. But rather than focus on subjective topics, I gave you concrete advice on the assumption that you would find my questioning of your design as an insult. People tend not to take "delete it all and start over" very well, so I reserve that for particularly awful code and real life subordinates. ;)

Quote:

Originally Posted by Narue

People tend not to take "delete it all and start over" very well, so I reserve that for particularly awful code and real life subordinates. ;)

Which means you would have been quite justified doing it in this case ;)

Quote:

Originally Posted by grumpy

Which means you would have been quite justified doing it in this case ;)

The code is that bad?

Besides the lack of comments, would you please tell me what needs to be done to derail it of its "awfully designed" state? ... Without using the C standard functions that this is intended to replicate?

#include <iostream>

using namespace std;





// CONSTANTS

const int MAX_SIZE = 256;





int get_size ( char text[MAX_SIZE] )

{

  int i = 0;



  while ( i < MAX_SIZE && text[i] != '\0' )

    ++i;



  return i;

}



int locate(char text[MAX_SIZE], char from[MAX_SIZE], 

           int size_f)

{

  int i2 = 0;

  

  for (int i1 = 0;i1 < MAX_SIZE;i1++)

  {

    if (text[i1] == from[i2])

    {

      i2 ++;

      if (i2 == size_f)

        return i1 - size_f + 1;

    }

    else if (text[i1] == '\0')

      return -1;

    else

      i2 = 0;

  } 

  

  return -1;

}



char *replace(char text[MAX_SIZE], char to[MAX_SIZE], 

              int where, int size_f, int size_t)

{

  char *new_text = new char[MAX_SIZE];



  int offs1 = size_f+where;

  int offs2 = size_t+where;

  

  // append text before change

  for (int i1=0; i1 <= where; i1++)

    new_text[i1] = text[i1];      



  // append change

  for (int i2=0; i2 <= size_t; i2++)

    new_text[i2+where] = to[i2];

  

  // append text after change

  for (int i3=0; i3 <= get_size(text)-size_f-where; i3++)

    new_text[i3+offs2] = text[i3+offs1];

  

  return new_text;

}



int main()

{

  char *text = new char[MAX_SIZE];

  char from[MAX_SIZE] = "some";

  char to[MAX_SIZE]   = "no";

  

  int size_f = get_size(from);

  int size_t = get_size(to);

  int where = 0;

  

  text = "This is some text, yaddi yaddi oh. some more text.";



  while (!(where == -1))

  {

    where = locate(text, from, size_f);

    if (!(where == -1))

      text = replace(text, to, where, size_f, size_t);

  }

  

  cout << text;

    

  cin.sync();

  cin.get();

  return 0;   

}

OK; just remember you asked.

I am ignoring the fact that we could probably do all this with a few lines using either C or C++ standard library features, and that your comments don't help much for explaining what your code is doing. I also ignore the fact that you are using the C++ standard library by using <iostream>.

1) Narue's already mentioned the issue of hard-coding array sizes in function declarations.

2) It's not really all that obvious, without a close look, what i1, i2, and size_f are being used for in your locate() function. That gives you a maintenance problem if you pick this code up in a few months and need to improve it.

3) You have multiple return points from your locate() function. Again, that gives you a maintenance problem if you pick this code up in a few months and need to improve it.

4) Minor nit (as compilers will cope with this): you're using postincrement when preincrement will do the job.

5) It is not obvious what the variables offs1, offs2, i1, i2, i3 are being used for in replace(). Yes, I know offs1 and offs2 are offsets, but that only is obvious after looking at the rest of the code.

6) The size of your new_text array (or memory allocated) needs to be larger than MAX_SIZE

7) size_t is a type that is used frequently in both the C and C++ standard libraries. You are using it as a variable name, which means obscure incompatibilities with either the C or C++ standard libraries .... and you are using <iostream>.

8) Your replace() function dynamically allocates memory, and requires the caller to release it. This is an opportunity for the caller to experience a memory leak. Your main function fully exploits that opportunity by calling replace() repeatedly in a loop, and never releasing the dynamically allocated memory.

9) Minor: "if (text[i1] == '\0')" can be simplified to "if (text[i])"

10) Why, in main(), declare from and to as being of size MAX_SIZE when you are assigning their contents using string literals?

11) You main() function declares text as a pointer, and assigns it to new char [MAX_SIZE]. Then you reassign text to a string literal, and therefore another memory leak.

12) "if (!(where == -1))" is more readably expressed as "if (where != -1)"

13) As Narue mentioned, cin.sync() and cin.get() probably aren't doing what you intend, even if it appears they are.

I haven't looked all that closely, so there are probably some other issues I've missed.