4
\$\begingroup\$

I wrote the following groovy function for taking a string of multiple lines (or any other character for splitting the code into parts), and bringing each line/part to a specified number of characters, cutting too long lines and filling up too short lines with a fill character:

static final String adjustLineLength(Integer length, String filler, String token, String source) {
    source
    .tokenize(token)
    .collect{ String line ->
        if(line.size() > length)
            line.substring(0, length)
        else if(line.size() < length)
            line + (filler * (length - line.size()))
        else
            line
    }
    .join(token)
}

Is that a good approach? Any suggestions how this code can be improved in concerns of making it more easy to understand and maybe (but that has not priority over beeing easy to understand) improving performance?

\$\endgroup\$
3
\$\begingroup\$

I think your approach is fine, and it's easy to understand. I have just a few improvement ideas:

  • A more natural ordering of the method parameters might be: source, token, length, filler
  • Instead of Integer, int should be enough and shorter
  • Move the closure in collect to its own method
  • Do you really need the static final String declaration? Why not use simply def?
  • Do you really need the type declarations in the signature? You could omit them, and it will be still quite clear
  • The brackets around the multiplication are unnecessary in

    line + (filler * (length - line.size()))
    

Suggested implementation

Putting the above suggestions together:

def adjustLineLength = { line, length, filler ->
    if (line.size() > length) {
        line.substring(0, length)
    } else if (line.size() < length) {
        line + filler * (length - line.size())
    } else {
        line
    }
}

def adjustTextLength = { source, token, length, filler ->
    source
    .tokenize(token)
    .collect { adjustLineLength(it, length, filler) }
    .join(token)
}
\$\endgroup\$
  • \$\begingroup\$ Hi, the reason why I used this "odd" ordering is because of currying. I want to use curry and if I order it like you posted (what I would do when not having to curry it) then I have to use ncurry and that is a little ugly. Any suggestions what to do in this case? Regarding the "verbose" syntax: I want it to be more familiar for java developers. But thx for the other points, they are really helpfull for me. =) \$\endgroup\$ – valenterry May 8 '14 at 20:15
  • \$\begingroup\$ I don't actually get your point about currying, but that doesn't mean anything, because I don't know groovy well at all (and so I cannot comment about the ugliness and ncurry). As for making it more java-developer-friendly, I would boldly go and embrace groovy, but that may be subjective ;-) \$\endgroup\$ – janos May 8 '14 at 21:00

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.