patterncppModerate
C++ Pig Latin program
Viewed 0 times
programlatinpig
Problem
I'm just wanting some input on to how others would approach this problem. I am learning and wish to gain insight on others techniques. Be as critical as you need. I want to learn. I feel this code is sloppy and the logic is confusing.
```
/*
Pig Latin
Write a program that reads a sentence as input and converts
each word to “Pig Latin.” In one version, to convert a word to
Pig-Latin you remove the first letter and place that letter at the
end of the word. Then you append the string “ay” to the word. Here
is an example:
English: I SLEPT MOST OF THE NIGHT
Pig Latin: IAY LEPTSAY OSTMAY FOAY HETAY IGHTNAY
*/
#include
#include
using namespace std;
// takes a string argument and returns the pigLatin equivalent
string pigLatin(string);
int main()
{
string mySentence;
getline(cin, mySentence);
mySentence = pigLatin(mySentence);
cout << mySentence << endl;
return 0;
}
string pigLatin(string word){
//pigLatWord holds word translated in pig latin.
//pigLatSentence holds entire translated sentence.
string pigLatWord, pigLatSentence = "";
int length = 0, index = 0;
while (word[index] != '\0'){
// .find returns -1 if no match is found
if (word.find(' ', index) != -1){
length = word.find(' ', index);
length -= index;//length - index = the number of characters in a word
pigLatWord = word.substr(index, length);
pigLatWord.insert(length, "ay");
pigLatWord.insert(length, 1, word[index]);//first letter is inserted at the end of the string
pigLatWord.erase(0, 1);// erase first letter in string
index += length + 1;//adding one moves index from 'space' to first letter in the next word
}
else{
pigLatWord = word.substr(index);
length = pigLatWord.length();
pigLatWord.insert(length, "ay");
pigLatWord.insert(length, 1, word[index]);
```
/*
Pig Latin
Write a program that reads a sentence as input and converts
each word to “Pig Latin.” In one version, to convert a word to
Pig-Latin you remove the first letter and place that letter at the
end of the word. Then you append the string “ay” to the word. Here
is an example:
English: I SLEPT MOST OF THE NIGHT
Pig Latin: IAY LEPTSAY OSTMAY FOAY HETAY IGHTNAY
*/
#include
#include
using namespace std;
// takes a string argument and returns the pigLatin equivalent
string pigLatin(string);
int main()
{
string mySentence;
getline(cin, mySentence);
mySentence = pigLatin(mySentence);
cout << mySentence << endl;
return 0;
}
string pigLatin(string word){
//pigLatWord holds word translated in pig latin.
//pigLatSentence holds entire translated sentence.
string pigLatWord, pigLatSentence = "";
int length = 0, index = 0;
while (word[index] != '\0'){
// .find returns -1 if no match is found
if (word.find(' ', index) != -1){
length = word.find(' ', index);
length -= index;//length - index = the number of characters in a word
pigLatWord = word.substr(index, length);
pigLatWord.insert(length, "ay");
pigLatWord.insert(length, 1, word[index]);//first letter is inserted at the end of the string
pigLatWord.erase(0, 1);// erase first letter in string
index += length + 1;//adding one moves index from 'space' to first letter in the next word
}
else{
pigLatWord = word.substr(index);
length = pigLatWord.length();
pigLatWord.insert(length, "ay");
pigLatWord.insert(length, 1, word[index]);
Solution
See at @Caridorc first.
Use the full power of the stream.
The standard stream operators will read a word at a time for you.
The above will read a file. If you just want to read a line. Read a line then use string stream.
DRY your code
Both sides of your if statement are doing the same thing. The difference is handling the last word (one not terminated by a space). You should move common code outside the
Declaration when needed.
Put your variable declarations as close to the point of use as you can. This way you don't need to look for their type very far (it is where the code is). They only get constructed when you need them (and destroyed as soon as possible) so preventing excessive use of space and not costing anything if you don't need them.
Declare one variable per line.
Humans need help in reading code. Don't make it hard for them. Declare one variable per line. This also helps when refactoring code later. You will only move the variables you need and source control tools will find it easier to do a diff on the change.
Pass by const reference.
If you do not plan on modifying the input pass by const reference to prevent a copy.
Design.
If I was actually going to write this I would encapsulate the concept of a
Use the full power of the stream.
The standard stream operators will read a word at a time for you.
std::string word;
while(std::cin >> word)
{
// DO STUFF.
}The above will read a file. If you just want to read a line. Read a line then use string stream.
// Read a line
std::string line;
std::getline(std::cin, line);
// convert line to stream
std::stringstream linestream(line);
// Loop over it one word at a time.
std::string word;
while(linestream >> word)
{
// Operate on word.
}DRY your code
Both sides of your if statement are doing the same thing. The difference is handling the last word (one not terminated by a space). You should move common code outside the
if statement just leave the non common code in the if statement.if ()
{
}
else
{
}
// This all seems common across the two if statements.
pigLatWord = word.substr(index);
length = pigLatWord.length();
pigLatWord.insert(length, "ay");
pigLatWord.insert(length, 1, word[index]);
pigLatWord.erase(0, 1);
index = word.length();Declaration when needed.
Put your variable declarations as close to the point of use as you can. This way you don't need to look for their type very far (it is where the code is). They only get constructed when you need them (and destroyed as soon as possible) so preventing excessive use of space and not costing anything if you don't need them.
// These could be moved to the other side of the if.
// As pigWord is only used after the loop (in my new version)
string pigLatWord;Declare one variable per line.
string pigLatWord, pigLatSentence = "";
int length = 0, index = 0;Humans need help in reading code. Don't make it hard for them. Declare one variable per line. This also helps when refactoring code later. You will only move the variables you need and source control tools will find it easier to do a diff on the change.
Pass by const reference.
If you do not plan on modifying the input pass by const reference to prevent a copy.
string pigLatin(string const& word){
// ^^^^^^Design.
If I was actually going to write this I would encapsulate the concept of a
Pig Latin word in a class. Then you can localize the change in a single well named class. You can then make use of standard routines to help read/print.class PigLatinWord
{
// STUFF
};
int main()
{
// Copies std::cin to std::cout converting to PigLatin
// on the copy to the output stream.
std::copy(std::istream_iterator(std::cin),
std::istream_iterator(),
std::ostream_iterator(std::cout, " ")
);
}Code Snippets
std::string word;
while(std::cin >> word)
{
// DO STUFF.
}// Read a line
std::string line;
std::getline(std::cin, line);
// convert line to stream
std::stringstream linestream(line);
// Loop over it one word at a time.
std::string word;
while(linestream >> word)
{
// Operate on word.
}if (<test>)
{
}
else
{
}
// This all seems common across the two if statements.
pigLatWord = word.substr(index);
length = pigLatWord.length();
pigLatWord.insert(length, "ay");
pigLatWord.insert(length, 1, word[index]);
pigLatWord.erase(0, 1);
index = word.length();// These could be moved to the other side of the if.
// As pigWord is only used after the loop (in my new version)
string pigLatWord;string pigLatWord, pigLatSentence = "";
int length = 0, index = 0;Context
StackExchange Code Review Q#91048, answer score: 14
Revisions (0)
No revisions yet.