0
\$\begingroup\$

I'm new to Groovy but have some experience with Java, C and Python. I'd appreciate your feedback on Groovy style and features in this FizzBuzz code.

Review Checklist

  • Is it idiomatic Groovy code?
  • Can anything be taken away?
  • Can it be more functional?
  • Could I make use of any Groovy-specific features?

FizzBuzzTest.groovy

import org.fizzbuzz.FizzBuzz
import org.junit.Before
import org.junit.Test

class FizzBuzzTest {

    FizzBuzz fb

    @Before
    void setUp() {
        fb = new FizzBuzz()
    }

    @Test
    void testGiven1Returns1AsString() {
        assert '1' == fb.toString(1)
    }

    @Test
    void testGiven2Returns2AsString() {
        assert '2' == fb.toString(2)
    }

    @Test
    void testGiven3ReturnsFizz() {
        assert 'Fizz' == fb.toString(3)
    }

    @Test
    void testGiven5ReturnsBuzz() {
        assert 'Buzz' == fb.toString(5)
    }

    @Test
    void testGiven15ReturnsFizzBuzz() {
        assert 'FizzBuzz' == fb.toString(15)
    }
}

FizzBuzz.groovy

package org.fizzbuzz

class FizzBuzz {

    String toString(number) {
        if (isMultipleOf3(number) && isMultipleOf5(number))
            return 'FizzBuzz'
        if (isMultipleOf3(number))
            return 'Fizz'
        if (isMultipleOf5(number))
            return 'Buzz'
        number.toString()
    }

    private boolean isMultipleOf5(number) {
        isMultipleOf.curry(5)(number)
    }

    private boolean isMultipleOf3(number) {
        isMultipleOf.curry(3)(number)
    }

    private def isMultipleOf = { int multipleOf, number ->
        number % multipleOf == 0
    }

    static void main(String... args) {
        def fb = new FizzBuzz()
        for (num in 1..100) {
            println fb.toString(num)
        }
    }

}
\$\endgroup\$
  • \$\begingroup\$ Code should follow the principle of least surprise, so language features should be used to increase clarity. I don't really see particular clarity in this code. \$\endgroup\$ – Martin Spamer Dec 22 '18 at 11:28
1
\$\begingroup\$

You could write a straightforward method:

private boolean isMultipleOf3(number) {
    isMultipleOf(3, number)
}

Or you could define it as a partial method:

private def isMultipleOf3 = isMultipleOf.curry(3)

But what you wrote is unnecessarily complicated:

private boolean isMultipleOf3(number) {
    isMultipleOf.curry(3)(number)
}
\$\endgroup\$
3
\$\begingroup\$

Referring to 'Could I make use of any Groovy-specific features?': You could use a Category to add a isMultipleOf() method to Integer. So you could write if (number.isMultipleOf3()) (see Groovy Goodness: Use Categories to Add Functionality to Classes)

class Multiple {
    static boolean isMultipleOf(Integer number, int multipleOf) {
        number % multipleOf == 0
    }
}

def foo = 5
use (Multiple) {
    if(foo.isMultipleOf(3))
        println "Fizz"
    else
        println "Buzz"
}
\$\endgroup\$

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.