3
\$\begingroup\$

Am I using the best conventions or are there better ways?

    static void makePigLatin(String text){

    String[] textArray = text.split(" ")
    String buffer, translated


    for (int i = 0; i < textArray.length; i++) {

        buffer = ""
        translated = ""

        for (int j = 0; j < textArray[i].length(); j++){

            if (textArray[i][j].find { a -> vowels.any {a.contains(it)}}) {

                translated = textArray[i].substring(j)
                break

            } else {

                buffer = "$buffer${textArray[i][j]}"
            }
        }

        if (textArray[i][textArray[i].length() - 1].find { a -> punctuation.any {a.contains(it)}}) {

            print "${translated.substring(0, translated.length() - 2)}$buffer" + "ay" + "${textArray[i][textArray[i].length() - 1]}" + " "
        } else {

            print "$translated$buffer" + "ay "
        }
    }

    println ""
}

The vowels and punctuation variables are global:

static def vowels = ["a", "e", "i", "o", "u", "y"]
static def punctuation = [",", ".", "?", "!", ":"]
\$\endgroup\$
  • \$\begingroup\$ I don't think this works correctly, I would think 'fxx' to be 'xxfay' but it is 'fxxay' here. I think the issue is that your code is checking for a vowel, not just the first vowel. \$\endgroup\$ – Captain Man Sep 12 '16 at 18:07
  • \$\begingroup\$ Yes there will be edge cases that I haven't gone through. This was just a quick exercise to get familiar with some Groovy functionality. Also, there aren't any words without at least one vowel :P \$\endgroup\$ – Lakshan Sivananthan Sep 12 '16 at 18:13
  • \$\begingroup\$ Why do you think there are no words without vowels? ;) \$\endgroup\$ – Captain Man Sep 12 '16 at 18:22
  • \$\begingroup\$ My vowel array includes 'y' haha. But yeah I didn't spend too much time on it. Just a quick implementation (y) \$\endgroup\$ – Lakshan Sivananthan Sep 12 '16 at 18:31
4
\$\begingroup\$

General

Strings

In Groovy you can use single quotes for "regular" strings. Documentation

buffer = ""
// equivalent to 
buffer = ''

It's unlike Java where single quotes are for characters.

One exception to this is when using the $variable in Strings, you have to use double quotes for this.

buffer = "$buffer${textArray[i][j]}"
// not equivalent to
buffer = '$buffer${textArray[i][j]}'

Also, when placing variables in Strings, I tend to always wrap in { } even when it's not needed.

buffer = "${buffer}${textArray[i][j]}"
// it's just more explicit

Method signature

This may be an unpopular opinion, but I like not using def in method signatures and instead using explicit types. It makes IDE suggestions and readability a lot better.

static void makePigLatin(String text){
// better than
static void makePigLatin(def text){

To me def is only okay inside of a method.

eachWithIndex

I actually just looked this up writing the answer. You can use a method called eachWithIndex on arrays in place of a closure. Documentation

for (int i = 0; i < textArray.length; i++) {
// or...
textArray.eachWithIndex { e, i ->
    // where i is the index and e is the textArray[i]
}

Scope

buffer and `translated are only ever used in your for loop. It's important to limit the scope of variables as much as possible. So it's probably better to declare them in the loop like so

// String buffer, translated
// remove above

for (int i = 0; i < textArray.length; i++) {

    String buffer = ""
    String translated = ""

Return a String instead of printing

Instead of your method being void make it return a String and remove all the println from it. Then from where you call makePigLatin instead do println makePigLating('some string here').

This limits the side-effects of the method. For tiny programs this is not a big deal but in larger projects knowing exactly what a method will do is very handy -- especially if the method is what is known as a "pure function" which means it literally has no side effects and its output is directly dependent on its output.

Consistent formatting

In some places you have spaces before braces (for (int i = 0; i < textArray.length; i++) {) and in others you don't (static void makePigLatin(String text){). It's less important whether or not you use them than how important it is to be consistent. Personally I put spaces before them.



Specifics

The below code is a little odd to me, it's saying in the ith word in the jth character, find each a in the jth character that is any of the vowels.

textArray[i][j].find { a -> vowels.any {a.contains(it)}}

No need to do a find on a single character. The below is a cleaner way to me.

vowels.any { vowel -> vowel.equals(textArray[i][j]) }

The below

buffer = "$buffer${textArray[i][j]}"

Can simply be

buffer += textArray[i][j]
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ That was a great answer, thank you very much for your feedback @Captain Man. I agree with returning a variable instead of just printing to console. I just thought it would be more efficient not to create a new variable for this. Same with putting the two variables buffer and translated inside the loop, it will recreate these two variables for every loop right? I figured that would be expensive, or at least less efficient. What do you think ? \$\endgroup\$ – Lakshan Sivananthan Sep 12 '16 at 15:05
  • \$\begingroup\$ @LakshanSivananthan Even if it is less performant in some tiny way I really wouldn't worry about it. This code is not going to be "mission critical". If your code is "fast enough" then it's as fast as it needs to be, only once it's too slow would I consider trying to speed it up and worry about little things like that. I mean, we should always try to use fast algorithms and not go into multiple loops for no reason, but worrying about where a variable is declared isn't going to save you but a few nanoseconds -- if even. \$\endgroup\$ – Captain Man Sep 12 '16 at 15:14
  • \$\begingroup\$ @LakshanSivananthan added a couple more suggestions \$\endgroup\$ – Captain Man Sep 12 '16 at 18:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.