storing values in an empty array

[Java2]

I enter 9 values into an empty array (no itnitialized data)
and try to output them at the end but it doesn't. I have managed to find that the data is just not geting stored there.

this is the code

#include <string>
#include <iostream>
#include <ctime>
#include <cstdlib>

using namespace std;

// +++++++++++++++++++++++variables++++++++++++++++++++++++++
char* letterArray[9] = {};
int i;
char* vowel;
char* consonant;
int ln;
string choice;
// ++++++++++++++++++++++Functions++++++++++++++++++++++++++++ 

// +++++Vowel function+++++
char* randomVowel()
{
//vowel Array
char* vowelArray[5] = {"A", "E", "I", "O", "U"};

//Random Generater
srand ((unsigned)time(0));
int vn;
vn = (rand()%5);

//assign to the array

//return
return vowelArray[vn];
}

// +++++Consonant function++++
char* randomConsonant()
{
//Consonant Array
char* consonantArray[21] = {"B", "C", "D", "F", "G", "H", "J", "K", "L", "M", "N", "P", "Q", "R", "S", "T", "V", "W", "X", "Y", "Z"};

//Random Generater
srand ((unsigned)time(0));
int vn;
vn = (rand()%20);

//return
return consonantArray[vn];
}

// Main Program 
int main()
{

//Ask the letter type and loop 9 times

for (i = 0; i < 9; )
{
cout << "Would you like a Vowel or a consonant? <V/C>\n"; 
cin >> choice;
if (choice == "v" || choice == "C")
{
           //get a random vowel
           cout << randomVowel() << "\n";
           vowel = randomVowel();
           vowel = letterArray[i];
           i++;
}
else if (choice == "c" || choice == "C")
{
     //get a random consonant
     cout << randomConsonant() << "\n";
     consonant = randomConsonant();
     consonant = letterArray[i];
     i++;
}
else
{
    cout << "That is not a valid request!\n";
}
}

// ouputting the letter array
for(ln = 0; ln < 9; ln++)
{
cout << "\n" << letterArray[ln];
}


return 0;
}

if i haven't explained this properly or are just wandering what the hell i am doing please don't hesitate to ask.

many Thanks

[kyoryu]

You don't appear to be ever assigning anything to your letterArray.

     consonant = randomConsonant();
     consonant = letterArray[i];

You're setting consonant to the returned value of randomConsonant() here, and then setting it to what's in the letterArray array at position i. I *think* what you wanted to do was:

     consonant = randomConsonant();
     letterArray[i] = consonant;

Either way, there's no reason for these to be char pointers rather than simple chars, that I can see. You're not storing strings. This actually goes for your vowel/consonant arrays as well... use the character form ('A') instead of the string form ("A") to store them.

And, frankly, since you're already using at least some STL, you're probably better off simply using a vector< char > instead of a raw C array.

There's also no reason for you to use any global variables in this program. Any variable you're using can be local to main(). Global variables should only by used when absolutely necessary.

And indent your code. It makes it easier to find errors. (apologies if the lack of indentation is a cut/paste issue).

With all due respect, it looks like this is homework. I'd suggest reading over the chapters a bit more... it looks like you're misunderstanding a couple of basic concepts. Learning what the code is actually doing in this case will help you out a lot over the long run. Build a house on a solid foundation, etc.

[Java2]

Thanks that worked.

Also your point about my use of code: I have just started learning C++ and just got to the chapter on arrays and functions and decided the best way to learn was to create my own program using these functions.

Anything which you feel i am not understanding or other things i should know please let me know.

[destin]

Well, I'll just point out that there's an easier way of going about getting random consonants and random vowels:

char randomConsonant() {
    srand ((unsigned)time(0));
    char c;
    do {
        c = (rand() % ('a' - 'z' + 1)) + 'a';
    } while ((c == 'a') || (c == 'e') || (c == 'i') || (c == 'o') || (c == 'u'));
    
    return c;
}

char randomVowel() {
    srand ((unsigned)time(0));
    char c;
    do {
        c = (rand() % ('a' - 'z' + 1)) + 'a';
    } while ((c != 'a') && (c != 'e') && (c != 'i') && (c != 'o') && (c != 'u'));
    
    return c;
}

[kyoryu]

Alrighty, then, let's look at a few things.

#include <string>
#include <iostream>
#include <ctime>
#include <cstdlib>

using namespace std;

// +++++++++++++++++++++++variables++++++++++++++++++++++++++
// first thing is that I'm getting rid of all of your global variables.  They're just 
// not needed.  If you find yourself using a global, see if there's a way to NOT 
// use one.  It's not that they're inherently bad, just most of the time.  Some 
// times, of course, globals are necessary, but you should always consider 
// them to be the exception.
//
// in general, variables should be declared in the innermost scope possible.
// "Scope" is defined roughly as "within a pair of curly braces."  A variable
// that leaves its pair of braces gets destroyed.  This is actually a GOOD thing.
//
// ex:
// {
//    int a;
// }
// a = 5; // DOES NOT COMPILE!  A is gone!

++++++++++++++++++++++Functions++++++++++++++++++++++++++++ 

// notice that I've done two things here.

// 1) I've changed the function to return a char, not a char *.  
// 2) I've used single instead of double quotes.
// Why did I make these changes?  Simply enough, your functions are dealing
// with characters, not strings.  At no point are you actually using anything
// as an actual string.  So, let's use basic characters instead.
// C does not have a string type.  What it has is null-terminated arrays of 
// characters.  These are normally passed around as char pointers, so a lot of 
// times it's easy to think "anything alpha is a char*".  But in your case, you 
// don't need the extra overhead, so let's not do it.
// I changed the characters in the array to single instead of double quotes, because that makes them chars (single values, 0-255) instead of c-style strings (null terminated arrays of characters).  In your previous code, you were actually storing "A\0" etc., instead of simply the letter A.

// +++++Vowel function+++++
char randomVowel()
{
    //vowel Array
    char vowelArray[5] = {'A', 'E', 'I', 'O', 'U'};

    //Random Generater
    srand ((unsigned)time(0));
    int vn;
    vn = (rand()%5);

    //assign to the array

    //return
    return vowelArray[vn];
}

// +++++Consonant function++++
char randomConsonant()
{
    //Consonant Array
    char consonantArray[21] = {'B', 'C', 'D', 'F', 'G', 'H', 'J', 'K', 'L', 'M', 'N', 'P', 'Q', 'R', 'S', 'T', 'V', 'W', 'X', 'Y', 'Z'};

    //Random Generater
    srand ((unsigned)time(0));
    int vn;
    vn = (rand()%20);

    //return
    return consonantArray[vn];
}

// Main Program 
int main()
{

//Ask the letter type and loop 9 times
    // create the letter array here, since it's not global any more.
    char letterArray[9];
    char choice; // again, let's just use a character.
    // and we'll go ahead and declare i in the for loop.  That'll
    // work fine on any C++ compiler I'm aware of, but it may not
    // work in pure C.
    for (int i = 0; i < 9; )
    {
        cout << "Would you like a Vowel or a consonant? <V/C>\n"; 
        cin >> choice;
        // have to use single instead of double quotes here as well.
        // chars can't equal a string, only a char.  "V" is a null terminated string
        // so in memory it looks like "V\0".  We just want a "V" by itself.
        if (choice == 'v' || choice == 'V')
        {
           // I'm going to go ahead and create the local
           // variable 'vowel' here.  It's a little inefficient, since
           // it'll get recreated multiple times, but creating variables in
           // the innermost scope possible helps to avoid weird problems
           // where you define/change a variable that you're not aware of.
           // Code that does what you want, and is safe, trumps *slightly*
           // inefficient code most of the time.  "Early optimization is the root
           // of all evil".  (NOTE:  That doesn't mean you should do obviously
           // horrificly slow things like bubble sorts).
           char vowel;
           //get a random vowel
           cout << randomVowel() << "\n";
           vowel = randomVowel();
           
           // need to reverse these here.  Remember, any time you're doing
           // an assignment (a = b), you're setting the value on the LEFT.  The 
           // value on the right is unchanged.
           letterArray[i] = vowel;
           i++;
        }
        // compare to char instead of string...
        else if (choice == 'c' || choice == 'C')
        {
             // create our variable.
             char consonant;
             //get a random consonant
             cout << randomConsonant() << "\n";
             consonant = randomConsonant();
             letterArray[i] = consonant;
             i++;
             // just a quick note here, though I didn't change anything.  In both of these cases
             // you're doing the same thing. You're grabbing a letter and shoving it in your array.
             // it might be a good idea to have your ifs just grab the letter, and then use the same
             // block of code to put it in the array.  In a small program like
             // this it *probably* doesn't matter, but it's a good idea to
             // avoid multiple blocks of code that do the same thing.  It
             // just makes changes later harder, because you have to 
             // know or remember to change two things instead of one.
        }
        else
        {
            cout << "That is not a valid request!\n";
        }
    }

    // ouputting the letter array
    for(int ln = 0; ln < 9; ln++)
    {
        cout << "\n" << letterArray[ln];
    }


return 0;
}

I haven't actually compiled and tested this, so it's possible that there may be bugs/syntax issues. Lemme know if you have more questions.

[destin]

Another approach:

#include <iostream>
#include <ctime>

using namespace std;

char randomConsonant() {
    srand ((unsigned)time(0));
    char c;
    do {
        c = (rand() % ('a' - 'z' + 1)) + 'a';
    } while ((c == 'a') || (c == 'e') || (c == 'i') || (c == 'o') || (c == 'u'));
    
    return c;
}

char randomVowel() {
    srand ((unsigned)time(0));
    char c;
    do {
        c = (rand() % ('a' - 'z' + 1)) + 'a';
    } while ((c != 'a') && (c != 'e') && (c != 'i') && (c != 'o') && (c != 'u'));
    
    return c;
}

int main() {
    const int SIZE = 9;
    char letterArray[SIZE];
    
    for (int i = 0; i < SIZE; i++) {
        char choice;
        cout << "Would you like a vowel or consonant? <V/C>" << endl;
        cin >> choice;
        
        if (toupper(choice) == 'C') {
            letterArray[i] = randomConsonant();
        } else if (toupper(choice) == 'V') {
            letterArray[i] = randomVowel();
        } else {
            cout << "That is not a valid request!" << endl;
            i--;
        }
    }
    
    for (int i = 0; i < SIZE; i++) {
        cout << endl << letterArray[i];
    }
}

[kyoryu]

This:

char randomVowel() {
    srand ((unsigned)time(0));
    char c;
    do {
        c = (rand() % ('a' - 'z' + 1)) + 'a';
    } while ((c != 'a') && (c != 'e') && (c != 'i') && (c != 'o') && (c != 'u'));
    
    return c;
}

is a BAD idea. It could theoretically hang forever if it doesn't just happen to hit a vowel. While it's practically unlikely to happen, it's not a good thing to put into code.

[Java2]

Thanks Kyoryu that was very helpful.

destin the do while code of getting a letter does not work. I can't remember the exact results but i think it was the consonants that did not work and still produced some vowels.