View Single Post
Old 02-26-2006, 10:40 AM   #5 (permalink)
kyoryu
Registered User
 
Join Date: Apr 2003
Posts: 34
kyoryu is on a distinguished road
Alrighty, then, let's look at a few things.

Code:
#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.
kyoryu is offline   Reply With Quote