After you enter a name, it just prints out "Would you like to convert another name? (y/n) " it doesn't even give the user a chance to enter anything and it doesn't print out the converted name. It just ends...Why?

#include<iostream>
#include<string>
using namespace std;

void conversion(char name[]);
void lowercase(char name[]);

int main()
{ 
char first[100], last[100];
char answer;
cout << "Enter a name (first then last): ";
cin >> first, last;
lowercase(first);
lowercase(last);
conversion(first);
conversion(last);

cout << "Would you like to convert another name? (y/n) ";
cin >> answer;

while (answer == 'Y' || answer == 'y')
{cout << "Enter a name (first then last): ";
cin >> first, last;
lowercase(first);
lowercase(last);
conversion(first);
conversion(last);
cout << endl;
cout << "Would you like to convert another name? (y/n)";
cin >> answer;
if (answer == 'N' || answer == 'n')
{break;}
}
return 0;
}

void conversion(char name[])
{



if (name[0] == 'a' || name[0] == 'e' || name[0] == 'i' || name[0] == 'o' || name[0] == 'u')
{toupper(name[0]);
cout << name << "way ";}

else {
for (int i = 0; i < 100; i++)
{ toupper(name[0]);
name = name + name[i];
}
cout << name << "ay ";}
}





void lowercase(char name[])
{
int i = 1;
while (name[i] != '\0')
{
name[i] = tolower(name[i]);
i++;
}
}

It probably is printing out the conversion but the next cout is done right after and overwriting it. You could try putting a linefeed in there.

char first[100], last[100];

Why use character arrays when you can use string objects? Change this to:

string first, last;


cin >> first, last;

Is incorrect. You're applying the comma operator here, which is why it compiles. The following will do what you're probably expecting:

cin >> first >> last;

You are using character arrays too for your functions. Also replace that with string objects. Note that you'll have to pass them by reference:

void lowercase(string& name)


int i = 1;

i is an index, but I assume you start at 1 on purpose because you want to keep the first letter's casing. In the function you do not check on the length of the string though. (e.g. what if the function somehow receives an empty string? The function could be applied general, in other projects)

toupper(name[0]);

You should assign the result of this to name[0] otherwise you're throwing it away.

if (name[0] == 'a' || name[0] == 'e' || name[0] == 'i' || name[0] == 'o' || name[0] == 'u')

You only check lower case with this. Aside it's a bit "meh".

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

The main reason why it's going wrong.. Why do you loop 100 times? It doesn't make sense. The body makes little sense too.

for (int i = 0; i < 100; i++)
        { 
            toupper(name[0]);
            name = name + name[i];
        }

What you want to do it move the first character to the end. As your function doesn't seem to want to modify the string, you needn't add "ay" to the string. Assuming you did bounds checking something like this would suffice:

name += name[0];
name.erase(0, 1);

Then your looping construct for the program itself is redundant. You also don't need

if (answer == 'N' || answer == 'n')
        {break;}

Use a do-while construction instead:

do
    {
        cout << "Enter a name (first then last): ";
        cin >> first >> last;
        lowercase(first);
        lowercase(last);
        conversion(first);
        conversion(last);
        cout << endl;
        cout << "Would you like to convert another name? (y/n)";
        cin >> answer;
    }
    while (answer == 'Y' || answer == 'y');

Applying those changes results in the following code which works, but still contains some "ugly" things. Your main question was "why doesn't it work" though, I guess that's answered..

#include<iostream>
#include<algorithm>
#include<string>
using namespace std;

void conversion(string name);
void lowercase(string& name);

int main()
{
    string first, last;
    char answer;

    do
    {
        cout << "Enter a name (first then last): ";
        cin >> first >> last;
        lowercase(first);
        lowercase(last);
        conversion(first);
        conversion(last);
        cout << endl;
        cout << "Would you like to convert another name? (y/n)";
        cin >> answer;
    }
    while (answer == 'Y' || answer == 'y');

    return 0;
}

void conversion(string name)
{
    // begins with vowel.
    if (name[0] == 'a' || name[0] == 'e' || name[0] == 'i' || name[0] == 'o' || name[0] == 'u')
    {
        // Add "way" to the end.
        name[0] = toupper(name[0]);
        cout << name << "way ";
    }

    // It begins with a consonant. Move it to the end and add "ay"
    else
    {
        name += name[0];
        name.erase(0, 1);

        cout << name << "ay ";
    }
}

void lowercase(string& name)
{
    int i = 1;
    while (name[i] != '\0')
    {
        name[i] = tolower(name[i]);
        i++;
    }
}

I actually made something something for this exersize some time ago for another forum. (Was probably a user from the same school?):

#include<iostream>
#include<algorithm>
#include<sstream>
#include<vector>
#include<string>
#include<iterator>
#include<exception>
#include<limits>

using namespace std;

string          ReadLine            (istream& instream);
vector<string>  Tokenize            (const string line);
bool            IsVowel             (const char symbol);
bool            IsConsonant         (const char symbol);
void            RunApplication      ();
void            ToPigLatin          (vector<string>& words);
string          ToPigLatin          (string word);
void            CopyCase            (const char from, char& to);
string          RemovePunctuation   (string s);

// Symbols considered vowerls.
const string VOWELS     = "aeiou";

// Symbols considered consonants.
const string CONSONANTS = "bcdfghjklmnpqrstvwxyz";

// Could set up a more proper exception construct, but "meh"..
class IOException: public exception
{
    virtual const char* what() const throw()
    {
        return "An I/O exception occurred.";
    }
} ioException;


string ReadLine (istream& instream)
{
    string line;

    if (!getline(instream, line))
    {
        throw ioException;
    }

    return line;
}


vector<string> Tokenize (const string line)
{
    stringstream ss(line);

    // Tokenize the string using istream_iterators.
    istream_iterator<string> it(ss);
    istream_iterator<string> end;

    // Sections of the line without the whitespaces.
    vector<string> words(it, end);

    // Further tokenize on non-alphanumeric things.
    // Not really bothered with doing this, so just remove them :3
    transform(words.begin(), words.end(), words.begin(), RemovePunctuation);

    return words;
}


string RemovePunctuation (string s)
{
    string::iterator it = remove_if(s.begin(), s.end(), ::ispunct);

    if (it != s.end())
    {
        s.erase(it);
    }

    return s;
}


void RunApplication()
{
    string line;
    bool runApplication;

    do
    {
        cout << "\nEnter a line to convert into pig-latin: ";

        // Obtain a line from stdin.
        try
        {
            line = ReadLine(cin);
        }
        catch(exception& e)
        {
            cerr << "An error occured on ReadLine. (\"" << e.what() << "\")\n";
            return;
        }

        // Tokenize the read strings.
        vector<string> words = Tokenize(line);

        if (words.size() > 0)
        {
            // Convert the tokens to pig-latin.
            ToPigLatin(words);

            // Show the translated line:
            cout << "The entered text translated to pig-latin: ";

            for (vector<string>::iterator it = words.begin(); it < words.end(); it++)
            {
                cout << *it << " ";
            }

            cout << endl << endl;
        }
        else
        {
            cout << "Nothing to convert!\n";
        }

        // Never mind checking for 'n'..
        cout << "Do you wish to convert another line of text? (y/n): ";

        runApplication = (tolower(cin.get()) == 'y');
        cin.ignore(numeric_limits<streamsize>::max(), '\n' );
    }
    while (runApplication);
}


void ToPigLatin(vector<string>& words)
{
    transform(words.begin(), words.end(), words.begin(), (string (*)(string))ToPigLatin);
}


void CopyCase (const char from, char& to)
{
    if (isupper(from))
    {
        to = toupper(to);
    }
    else
    {
        to = tolower(to);
    }
}


string ToPigLatin(string word)
{
    if (word.length() > 0)
    {
        if (IsVowel(word.at(0)))
        {
            return word + "way";
        }
        else if (IsConsonant(word.at(0)))
        {
            // If there's more than 1 character, copy casings. The character that is moved to the back
            // depends on the current last character, and the new front character depends on the current from character.
            if (word.length() > 1)
            {
                CopyCase(word[0], word[1]);
                CopyCase(*word.rbegin(), word[0]);
            }

            // Let the prefix depend on the final character as well. No good rules were supplied for this.
            if (isupper(*word.rbegin()))
            {
                return word.substr(1) + word[0] + "AY";
            }
            else
            {
                return word.substr(1) + word[0] + "ay";
            }
        }
    }

    return word;
}


bool IsVowel (const char symbol)
{
    return VOWELS.find(tolower(symbol)) != string::npos;
}


bool IsConsonant (const char symbol)
{
    return CONSONANTS.find(tolower(symbol)) != string::npos;
}


int main()
{
    cout << "Pig-latin converter.\n";

    RunApplication();

    return 0;
}

Edited 3 Years Ago by Gonbe

Comments
Quality post.

I changed my program after doing some reading on substrings. I don't get a build error, but when I start the program it says "string subscript out of range."

#include<iostream>
#include<string>
using namespace std;

void conversion(string name);
void lowercase(string n);

int main()
{ 
string first, last;
char answer;
do
{cout << "Enter a name (first then last): ";
cin >> first, last;
lowercase(first);
lowercase(last);
conversion(first);
conversion(last);
cout << "Would you like to convert another name? (y/n) ";
cin >> answer;
}
while (answer == 'Y' || answer == 'y');

return 0;
}

void conversion(string name)
{
string way, ay, consonants;

if (name[0] == 'a' || name[0] == 'e' || name[0] == 'i' || name[0] == 'o' || name[0] == 'u')
{toupper(name[0]);
name = name + way;
cout << name;}


else {
    for (int i = 1; i < (int)name.length(); i++)
    {if (name[i] == 'a' || name[i] == 'e' || name[i] == 'i' || name[i] == 'o' || name[i] == 'u')
    {   consonants = name.substr(0, (i - 1));
        name = name.substr(i);
        toupper(name[i]);
        name = name.substr(i) + consonants + ay;
        cout << name;}
    }

}
}

void lowercase(string n)
{
int i = 1;
while (n[i] != '\0')
{
n[i] = tolower(n[i]);
i++;
}
}

Edited 3 Years Ago by munchlaxxx

In line 40 when I is 0 your passing -1 in the second parameter, which if I'm not mistaken is supposed to be the length of the substring.

Edited 3 Years Ago by tinstaafl

You didn't quite understand the substring member function. There might also be something wrong with your overall design/logic. But maybe you should describe it in words. It could be it does make sense but you just didn't manage to translate it correctly to code. Here's your code with some comments and some modification:

#include<iostream>
#include<string>
using namespace std;

void conversion(string name);
void lowercase(string& n);

int main()
{
    string first, last;
    char answer;
    do
    {
        cout << "Enter a name (first then last): ";

        // Still wrong.. replaced.
        //cin >> first, last;
        cin >> first >> last;

        lowercase(first);
        lowercase(last);

        conversion(first);
        conversion(last);

        cout << "\nWould you like to convert another name? (y/n) ";
        cin >> answer;
    }
    while (answer == 'Y' || answer == 'y');

    return 0;
}

void conversion(string name)
{
    // none needed.
    //string way, ay, consonants;

    if (name[0] == 'a' || name[0] == 'e' || name[0] == 'i' || name[0] == 'o' || name[0] == 'u')
    {
        // This statement will do "nothing", removed.
        //toupper(name[0]);

        // 'way' is uninitialized. Either use the desired value directly or initialize way accordingly. I did the first below.
        //name = name + way;

        name = name + "way ";
        cout << name;
    }
    else
    {
        cout << name.substr(1) << name[0] << "ay ";
        // You don't really need to go past every character, removed
        /*
        for (int i = 1; i < (int)name.length(); i++)
        {
            // I don't know what you're trying to do here. It doesn't matter what the other characters are.

            // Removed completely
            if (name[i] == 'a' || name[i] == 'e' || name[i] == 'i' || name[i] == 'o' || name[i] == 'u')
            {
                // This code makes little sense.
                consonants = name.substr(0, (i - 1));
                name = name.substr(i);
                toupper(name[i]);
                name = name.substr(i) + consonants + ay;
                cout << name;
            }
        }
        */
    }
}

// You pass it by value. Changes to n will remain local. Change it to call by reference, like i said.
//void lowercase(string n)
void lowercase(string& n)
{
    // Don't check specific indexes for '\0', especially not when you don't confirm you can access that index.
    // Use the size member function instead.
    for (unsigned int i = 1; i < n.size(); i++)
    {
       n[i] = tolower(n[i]);
    }
}
This article has been dead for over six months. Start a new discussion instead.