patterncsharpMajor
Simple Pig Latin Translator
Viewed 0 times
translatorsimplelatinpig
Problem
The goal of my assignment is simple:
Write a program that converts a given text to "Pig Latin". Pig Latin
consists of removing the first letter of each word in a sentence and
placing that letter at the end of the word. This is followed by
appending the word with letters "ay".
Example
Input: THIS IS A TEST
Output: HISTAY SIAY AAY ESTTAY
I want to know if there is any way to write this part of the code in a different/better way:
Here is my full code:
Write a program that converts a given text to "Pig Latin". Pig Latin
consists of removing the first letter of each word in a sentence and
placing that letter at the end of the word. This is followed by
appending the word with letters "ay".
Example
Input: THIS IS A TEST
Output: HISTAY SIAY AAY ESTTAY
I want to know if there is any way to write this part of the code in a different/better way:
foreach (string word in engword.Split())Here is my full code:
string engword = textBox1.Text; //english word
string pig1 = ""; //pig latin
string pig2 = ""; //first letter
string space = " ";
string extra = ""; //extra letters
int pos = 0; //position
foreach (string word in engword.Split())
{
if (pos != 0)
{
pig1 = pig1 + space;
}
else
{
pos = 1;
}
pig2 = word.Substring(0,1);
extra = word.Substring(1, word.Length - 1);
pig1 = pig1 + extra + pig2 + "ay";
}
MessageBox.Show(pig1.ToString());
}
}
}Solution
This looks pretty decent for a beginner. Some suggestions:
-
Making a variable
-
Why is pos an integer? it only has two values, and you are using it to test a condition. Use a bool instead.
-
For that matter, why have pos in the first place? Pos tells you the same thing as
-
Any time you write a comment, ask yourself did I add this comment because the code was unclear? and then can I write the code so clearly that I don't need the comment? You say
Dude. If the name of the variable is
-
pig2 is just one letter, so it can be a
-
You're using
-
Finally, like I said, this is fine for a beginner, walk before you run, and so on. To give you a sense of how an expert would write this code, I'd write it like this:
Which is nicely compact.
-
Making a variable
space instead of using the literal directly is actually a kind of nice idea. You can improve the code by marking the local const.-
Why is pos an integer? it only has two values, and you are using it to test a condition. Use a bool instead.
-
For that matter, why have pos in the first place? Pos tells you the same thing as
pig1.Length == 0.-
Any time you write a comment, ask yourself did I add this comment because the code was unclear? and then can I write the code so clearly that I don't need the comment? You say
string pig2 = ""; // First letterDude. If the name of the variable is
firstLetter then you don't need the comment.-
pig2 is just one letter, so it can be a
char instead of a string. You can say:char firstLetter;
...
firstLetter = word[0];-
You're using
pig1 as an accumulator; you're accumulating the final result piece by piece. This is totally fine for small strings. If that string was really big then this is not an efficient technique; you should use StringBuilder instead if you want to make an accumulator for a large string.-
Finally, like I said, this is fine for a beginner, walk before you run, and so on. To give you a sense of how an expert would write this code, I'd write it like this:
string pigLatin = string.Join(" ",
engword.Split()
.Select(word => word.Substring(1, word.Length - 1) + word[0] + "ay"));Which is nicely compact.
Code Snippets
string pig2 = ""; // First letterchar firstLetter;
...
firstLetter = word[0];string pigLatin = string.Join(" ",
engword.Split()
.Select(word => word.Substring(1, word.Length - 1) + word[0] + "ay"));Context
StackExchange Code Review Q#24841, answer score: 20
Revisions (0)
No revisions yet.