Pig Latin Code, looks professional?

1

Hey there,

I solved the pig latin challenge.

It works but I wonder if the code is too sloppy. What do you all think? Is this bad code?

function translatePigLatin(str) {
  var strArr = str.split("");
  var vowels = ["a", "e", "i", "o", "u"];
  var newArr = [];
  for (i = 0; i < strArr.length; i++) {
    for (j = 0; j < vowels.length; j++) {
      if (strArr[0] === vowels[j]) {
        return str  + 'way';
      } 
      else if (strArr[i] === vowels[j]) {
        return str.substr(i) + newArr.join('') + 'ay';
      }
    }
    newArr.push(strArr[i]);
  }
}

translatePigLatin(“california”);

2

It works, plus it’s not hard to understand. It’s not bad code :slight_smile:

You can eliminate the inner for-loop by using includes().

  for (i = 0; i < strArr.length; i++) {
    if (vowels.includes(str[0])) return str  + 'way';
    if (vowels.includes(str[i])) return str.substr(i) + newArr.join('') + 'ay';
    
    newArr.push(strArr[i]);
  }

Or you can go the regex route.

  var vowels = /[aeiuo]/;
  for (i = 0; i < strArr.length; i++) {
    if (vowels.test(str[0])) return str  + 'way';
    if (vowels.test(str[i])) return str.substr(i) + newArr.join('') + 'ay';
    
    newArr.push(strArr[i]);
  }

You didn’t have to create the strArr array. You can directly access characters from a string as if they were arrays themselves.

  for (i = 0; i < str.length; i++) {
    for (j = 0; j < vowels.length; j++) {
      if (str[0] === vowels[j]) {
// ...

If you’re interested, check out this fantastic one-liner for Pig Latin.

1 Like
3

The idea in coding is to make it look as simple as possible, So one of kevcomedia’s first two solutions get the big tick from me. However, imho the “fantastic one-liner” maybe “clever” and great for a cryptic competition, but bad practice for general coding. Imagine you’re modifying someone elses code and you come across this - it would take (me) ages to work out what it does.

1 Like
4

Well, what a way to be summoned. Now that I am here though, I may as well give my two cents on the subject.

@nikrb, you are of course completely right though I am quite certain you already know that. You have that debonair feeling around you. It is naturally true that using a js method in the way it was supposed to be used is a crime and I should be verbally abused for it.

Furthermore, I feel I should warn you about other such chicanery. This is why I will link another two examples of similar garbage code written by “clever” people:

create-react-app

jQuery

The first one is from create-react-app and is written by the same people who develop React. The second one is from jQuery. I’d advise you to stay far away from these garbage libraries and forgo the use of jQuery and React in your code.

You should instead try to adopt the imperative solutions that you could so “easily” read. You “understood” the code in those so well, you actually failed to notice that for certain cases these solutions would return the helpful undefined instead of a string containing either the original input or the original input with ‘ay’ at the end (the challenge rules don’t really specify what should happen but these are cases that certainly appear when chatting or texting). Once that happens and you attempt to do anything with the data you will inevitably get the even more helpful uncaught type error that is usually associated with such “well” written code.

1 Like
5

You could also avoid loops entirely. This worked for me.

function translatePigLatin(str) {
  var vow=["a","i","e","o","u"]; //vowels
  
  var firstVow = str.split("").map(function (x) { //find first vowel of string
    return vow.indexOf(x) !== -1 ? 1 : 0;
  }).indexOf(1);
    
  return firstVow===0 ? str+"way" : str.substr(firstVow)+str.substr(0,firstVow)+"ay"; //return pig latin
}
6

Every challenge I find the solution thinking it’s short and great, just to come here and find that someone solved it in like one line. hahahaha.

function translatePigLatin(str) {
  //check if first letter is a vowel
  var isVowel = str.slice(0,1);
  
  function letter(isVowel) {
    var result;
    result = isVowel == "a" || isVowel == "e" || isVowel == "i" || isVowel == "o" || isVowel == "u";
    return result;
  }
  
  if (letter(isVowel)){
    return str + "way";
  } else if (letter(str.slice(0,1)) ===  false && letter(str.slice(1,2)) === false){
    return str.slice(2) + str.slice(0,2) + "ay";
  } else if (letter(isVowel) === false) {
    return str.slice(1) + str.slice(0,1) + "ay";
  } 
}

translatePigLatin("glove");
7

I am a professional (20+ years getting paid to code). Here’s my opinion, for what it’s worth.

First and foremost, good job on solving the challenge! This is always the most important first step in the process.

To quote Kent Beck, the process is:

“Make it work. Make it right. Make it fast.”

It seems obvious that working code should be the first step, but you’d be surprised at how often developers get caught up in trying to “make it right” before they’ve even made it work.

So what does “make it right” mean? How do we know that code is “right”?

Code is “right” when it works as expected and is easy to change. Code is easy to change when it’s easy to read and understand. And code is easy to read understand when it has the following properties:

  • clear, honest, and descriptive variable and function names
  • descibes what it is doing, moreso than how it is doing it
  • is unencumbered by dependencies
  • has fewer steps, but not so few that it becomes hard to follow

If I were refactoring your code, I would strive to eliminate the nested for loops and complex if/else-if statement. I would be looking to end up with something like:

function translatePigLatin(word) {
  var letters = word.split('');
  var preConsonants = [];
 
  while(letters.length > 0 &&
        !'aeiou'.includes(letters[0])) {
    preConsonants.push(letters.shift());
  }
  
  return letters.concat(preConsonants).join('') +
         ((preConsonants.length > 0) ? 'ay' : 'way');
}

Hope that helps.

Let me know if you have any questions.

9

@rmdawson71 I stopped at “make it work”. Looks like you got to “make it fast”. I like the use of match and indexOf to eliminate the explicit loop. Very nice!

P.S. [SOLVED] You have a small typo on line 4: index should be vowelPosition