Anything wrong?

[Linette]

Hi,
I've just written some code, but I don't have a compiler on this computer to check if it's right. Can anyone tell me if there's anything wrong? (i.e. illegal operations, etc?)

Thanks a lot.

//these three lines will be in main
char original [20]; 
cin.getline (original,20);
char *newstring = new char [20];
newstring = encrypted (original); //particularly this line

//this is to be outsie of main.. some function for itself..
char *encrypted (char original[20]) {

  char enc [20];
  int c;

  for (c=0; c<20; c++) {
     if ( int(original[c])+1 <= 90 )
        enc[c] = char(int(original[c])+1);
     else
        enc[c] = char(int(original[c])-25);  
  }

  strcpy (encrypted, enc);
  return (encrypted);

} 

[Unregistered]

the return from encrypted is a char * as is newstring, however, the pointers are being used as strings. Therefore, I would recommend you use strcpy to place return of encrypted into newstirng, rather than the assignment operator:

strcpy(newstring, encrypted(original));

[Linette]

ok that makes sense. great, thanks..
so the rest will be fine then?
and um, is that what you always do when your function's returning a char *?

[Unregistered]

Well, I try not to return a char * if I can help it (i usually send string to be changed by reference so I don''t have to "return" it at all), but if there is no other option, then yes, you have to use strcpy to copy c_style strings, you can't use the assignment operator.

//my version might look more like this:

char original [20];
cin.getline (original,20);
char *newstring = new char [20];
strcpy(newstring, original);
encrypted (newstring);
cout << newstring;

//this is to be outsie of main.. some function for itself..
void encrypted (char * newstring) {
//etc.

[Unregistered]

strcpy (encrypted, enc);
return (encrypted);

The above lines don't make too much sense as you are trying to copy a string into a function....won't work. If you wish to continue returning a string then it would be just:

return enc;

without the strcpy().

Also, enc is local to encrypted() and that might cause a problem returning a pointer to a local function variable becaiuse when the function is completed and the local function variable goes out of scope what will the pointer point to? So overall, I would try to avoid returniing pointers to variables local to functions other than main.


Oh, and what happens if string entered is only 10 char long rather than twenty? Try using strlen like this:


for (c=0; c < stlen(original); c++) {
//ecnryption code;
}

so you aren't using any undefined elements of original or newstring or whatever string you end up working with in the loop.

ooh.....ooh.....and then you need to put a null terminating char at the end of enc after the loop is done by adding this line:

enc[c] = '\0';

which you wouldn't have to do if you pass newstring by reference...

but then it looks like it might work.