Setting pointer to the return value of a function

Question

I'm trying to set the variables char * vowels and char * consonants as the return value of the functions searchVowels and searchConsonants respectively.

Although when I test the code, the above variables are getting set properly but not being passed back into the main. And during a test with

cout << "text vowels" << vowels << "sametext" << consonants; ///something like this.

it doesn't display the consonant value now.

Here's my code, any suggestions would be super helpful. Except that I can't use strings.(For a class)

Also is this the appropriate way to post code?

 #include <iostream>                                                             
 #include <cctype>                                                               
 #include <cstring>                                                                   
 using namespace std;                                                                                                                                         
 const int SIZE = 7;                                                             


 //This function greets the user.                                                
 void greetUser();                                                               

 //This funcion asks for the array of letters.                                   
 char * inputLetters(char * inputArray);                                         

 //This will capitalize all letters to make them easier for the computer         
 //to compare.                                                                   
 char * capitalizeLetters(char * inputArray);                                    

 //This function will search the array for vowesl. If no vowels no game.         
 char * searchVowels(char * arrayCopy);                                          

 ///This function will search the array for consonants.                          
 char * searchConsonants(char * arrayCopy);                                      


 //This capitalizes all the letters in the initial array.                        
 char * capitalizeLetters(char * inputArray)                                     
 {                                                                               
    for (int i = 0; i < 6; ++i)                   
                    {                                                                       
            inputArray[i] = toupper(inputArray[i]);                         
    }                                                                       
 //      inputArray = toupper(inputArray);                                       
    return inputArray;                                                      
 }                                                                               


 //This program will search the array for consonants                             
    //and return the consonants array.                                      
 char * searchConsonants(char * arrayCopy)                                       
 {                                                                               

    char * consonants; consonants = new char[SIZE];                         

    for (int i = 0; i < 6; ++i)                                             
    {//I feel like I could make this into a function itself...hmm           
            if( arrayCopy[i] != 'A' && arrayCopy[i] != 'E'                  
            && arrayCopy[i] != 'I' && arrayCopy[i] != 'O' &&                
            arrayCopy[i] != 'U' && arrayCopy[i] != 'Y')                     
            {                                                               
            consonants[i] = arrayCopy[i];                                   
            }                                                               
    }                                                                       

 return consonants;                                                              

 }

There's so much wrong here. It looks like you're using C++, but very broken C++. For instance, you cannot omit the return type of `main`, and you appear to be mixing `new` and `free` which is asking for trouble. I suggest finding a [good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) — Tas, Feb 05 '18 at 04:30
`if( arrayCopy[i] == 'A' && arrayCopy[i] == 'E'` Clearly, the same character can't be equal to both `'A'` and `'E'` at the same time. The condition is never true. — Igor Tandetnik, Feb 05 '18 at 04:31
I feel like I should just post the whole thing. I truncated a lot of it.... — vvbudh, Feb 05 '18 at 04:41
Best to stick to one problem per question. Pick one problem and make a small program that demonstrates that one problem. We call this a [mcve] The beauty f the MCVE is in making one you very often expose the problem for what it is and figure out how to fix it. Oh and `return` ends the function, so `return arrVowels; free (arrVowels);` never reaches the `free(arrVowels);`. This also exposes a problem with trying to `return` and `free` you free stuff ans obviously it's gone. You'll find better ways to do this, like `std::string`, in the future. — user4581301, Feb 05 '18 at 04:59

Vishaal Shankar · Accepted Answer · 2018-04-04T07:14:10.190

In the method searchVowels, you seems to have the following code :

        if( arrayCopy[i] == 'A' && arrayCopy[i] == 'E'                  
        && arrayCopy[i] == 'I' && arrayCopy[i] == 'O' &&                
        arrayCopy[i] == 'U' && arrayCopy[i] == 'Y')                     
        {                                                               
                arrVowels[i] = arrayCopy[i];                            
        }

How are you expecting the arrayCopy[i] to pass the check since it cannot have all vowels at the same time. I think you're looking for an OR check here.

        if( arrayCopy[i] == 'A' || arrayCopy[i] == 'E'                  
        || arrayCopy[i] == 'I' || arrayCopy[i] == 'O' ||
        arrayCopy[i] == 'U' || arrayCopy[i] == 'Y')                     
        {                                                               
                arrVowels[i] = arrayCopy[i];                            
        }

In the above case, it might fill the arrayVowels with something if the check passes.

Also, you can make the above code into a function something like HasVowels(), which checks if the the arrayCopy has a vowel at the ith index and then use it in both searchVowels and searchConsonants.

One more thing is the usage of "free" in your code. In C++, delete operator should only be used either for the pointers pointing to the memory allocated using new operator, and free() should only be used either for the pointers pointing to the memory allocated using malloc() or for a NULL pointer.

Okay so I won't use free() then if I don't use malloc(); Thanks mate Also && switch to || got it. — vvbudh, Feb 05 '18 at 04:49

A B · Answer 2 · 2018-02-05T05:26:54.953

The new operator should be used with the delete operator (not with free).

You should not delete or free memory that you're returning from a function if the return value is intended for use by the caller.

In this example below, the caller of the function (not the function itself) allocates the memory and the caller of the function frees the memory when it is no longer needed.

The searchVowels function won't necessarily know when the caller no longer needs the memory.

In this example below, the searchVowels function does not allocate memory and assumes that memory for the dest argument array and the input string have already been allocated.

/* IMPORTANT: This code assumes that SIZE is already defined */

void searchVowels(char* dest, char * inputString) {                                          
    int destIndex;
    int i;

    /* Update this function with other functionality */
    /* as intended */

    destIndex = 0;
    dest[destIndex] = '\0';

    for(int i = 0; i < strlen(inputString); i++)                                              
    {                                                                       
        if ((inputString[i] == 'A') || (inputString[i] == 'E')) {
        {   
            if (destIndex < SIZE) {
                dest[destIndex] = inputString[i];
                dest[destIndex+1] = '\0';
                destIndex = destIndex + 1;
            }                                                            
        }                                                               
    }                                                                       
}  

/* From the code that calls searchVowels */

char* result;

try {
    char* result = new char[SIZE];

    searchVowels(result, "TESTAEIOU");

    /* Use the result variable Here */

    delete result;

} catch (std::bad_alloc&) {
  /* new did not allocate memory */
}

So I should be putting in two args for my searchConsonant function? Should I not be trying to return the data and instead set it equal to destIndex[i] and pass it instead by refrence? And does the destIndex make sure I don't add chars I don't want into the pointer? — vvbudh, Feb 05 '18 at 05:17
Hi This function uses 2 arguments because it assumes that both the input and destination already have the memory allocated. This function (as written) does not allocate memory. In that sense, the responsibility for allocating the memory and deallocating the memory lies with the calling function. Using this as a convention makes it easier to know which parts of the code should allocate and deallocate. — A B, Feb 05 '18 at 05:22
Yes - if this is used as a string it should be null-terminated - I added that also — A B, Feb 05 '18 at 05:27

score 0 · Answer 3 · answered Feb 05 '18 at 16:57

I fixed the issue, it was returning but not setting the data how it should have been. I needed to use the || pipes for the vowels and set up another for loop to check all the conditions. It had nothing to do with freeing up memory. Here's the code of the function.

     char * searchConsonants(char * arrayCopy)                                       
{                                                                               

    char * consonants; consonants = new char[SIZE];                         

    for (int i = 0; i < 6; ++i)                                             
    {//I feel like I could make this into a function itself...hmm           
            for(int j = 0; j < 6; ++j)                                      
            {                                                               
                    if( arrayCopy[j] != 'A' && arrayCopy[j] != 'E'          
                    && arrayCopy[j] != 'I' && arrayCopy[j] != 'O' &&        
                    arrayCopy[j] != 'U' && arrayCopy[j] != 'Y')             
                    {                                                       
                    consonants[i] = arrayCopy[j];                           
                    ++i;                                                    
                    }                                                       
            }                                                               
    }                                                                       

return consonants;                                                              
}

Setting pointer to the return value of a function

3 Answers3