HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMajor

Simple Pig Latin Translator

Submitted by: @import:stackexchange-codereview··
0
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:

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 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 letter


Dude. 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 letter
char 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.