Loop vs closure, readable vs concise?

Question

In my answer to my own question here I posted some code and @Dave Newton was kind enough to provide me with a gist and show me the error in my not-so-Groovy ways. <-- Groovy pun

I took his advice and revamped my code to be Groovier. Since then the link I am making (which Dave represents with the replaceWith variable) has changed. Now the closure representation of what I want to do would look like this:

int i = 1
errorList = errorLinksFile.readLines().grep { it.contains "href" }.collect { line ->
    def replaceWith = "<a href=\"${rooturl}${build.url}parsed_console/log_content.html#ERROR${i++}\">"
    line.replaceAll(pattern, replaceWith).minus("</font>")
}

And the for loop representation of what I want to do would look like this:

def errorList = []
def i = 1
for(line in errorLinksFile.getText().split("\n")){
    if(!line.contains("href")){
        continue
    }
    errorList.add(line.replaceAll(pattern, "<a href=\"${rooturl}${build.url}parsed_console/log_content.html#ERROR${i++}\">").minus("</font>"))
}

The closure version is definitely more concise, but I'm worried if I always go the "Groovier" route the code might be harder for other programmers to understand than a simple for loop. So when is Groovier better and when should I opt for code that is likely to be understood by all programmers?

Using closures is idiomatic (and elegant) with Groovy. A programmer is not a Groovy programmer if they don't understand closures. — SteveD, Jan 06 '12 at 15:26
Using `grep` and `collect` chained like that might be a bit much (and if I had time to think about the problem I might do something entirely different), but as @SteveD says, this is (closer to) idiomatic Groovy (same with other similar languages). Better to educate the devs--it can lead to better Java code as well, IMO. — Dave Newton, Jan 06 '12 at 15:39
(I'd also put that lonely `i++` in the interpolated string, the poor thing.) — Dave Newton, Jan 06 '12 at 15:40
@SteveD My company deals mostly with embedded development, so most of my co-workers are not Groovy developers, thus my concern. Maybe it is best to stretch their comfort zone a little though :) — ubiquibacon, Jan 06 '12 at 15:45
@typoknig I like to stretch people's comfort zones a little, get them to think outside of the box, but maybe that's just me :) — Jarred Olson, Jan 06 '12 at 15:54
If they're embedded folks, make them learn Forth. That'll mess with their heads. Learning additional models of abstraction almost invariably leads to better decisions/architectures regardless of the language ultimately used. — Dave Newton, Jan 06 '12 at 16:03

score 2 · Accepted Answer · answered Jan 06 '12 at 15:40

I believe that a development team should strive to be the best and coding to the least knowledgeable/experienced developer does not support this. It is important that more than one person on the team knows how to read the code that is developed though. So if you're the only one that can read it, teach someone else. If you're worried about someone new to the team being able to read it I feel that they would be equally hard to read since there would be lack of domain knowledge. What I would do though is break it up a little bit:

def originalMethod() {
    //Do whatever happens before the given code
    errorList = getModifiedErrorsFromFile(errorLinksFile)
}

def getModifiedErrorsFromFile(errorLinksFile) {
    int i = 1
    getHrefsFromFile(errorLinksFile).collect { line ->
        def replaceWith = getReplacementTextForLine(i)
        i++
        line.replaceAll(pattern, replaceWith).minus("</font>")
    }
}

def getHrefsFromFile(errorLinksFile) {
    errorLinksFile.readLines().grep { it.contains "href" }
}

def getReplacementTextForLine(i) {
    "<a href=\"${rooturl}${build.url}parsed_console/log_content.html#ERROR${i}\">"
}

This way if the next person doesn't immediately understand what is going on they should be able to infer what is going on based on the method names. If that doesn't work adding tests would help the next person understand what is going on.

My 2 cents. Good topic though!!!

score 1 · Answer 2 · answered Jan 17 '12 at 03:31

1

Idiomatic groovy is good, people will learn the common idioms quickly. "Clever" groovy, in my opinion, is more likely to be just confusing.

answered Jan 17 '12 at 03:31

Michael Rutherfurd

13,815
5
29
40

Loop vs closure, readable vs concise?

2 Answers2