Is There a Groovier Way To Add Dashes to a String?

Question

I have the following code, which works, but I'm wondering if there is a "groovier" way of doing this:

/**
   * 10 digit - #-######-##-#
   * 13 digit - ###-#-######-##-#
   * */
private formatISBN(String isbn) {
  if (isbn?.length() == 10) {
    def part1 = isbn.substring(0, 1)
    def part2 = isbn.substring(1, 7)
    def part3 = isbn.substring(7, 9)
    def part4 = isbn.substring(9, 10)
    return "${part1}-${part2}-${part3}-${part4}"
  } else if (isbn?.length() == 13) {
    def part1 = isbn.substring(0, 3)
    def part2 = isbn.substring(3, 4)
    def part3 = isbn.substring(4, 10)
    def part4 = isbn.substring(10, 12)
    def part5 = isbn.substring(12, 13)
    return "${part1}-${part2}-${part3}-${part4}-${part5}"
  } else {
    return isbn
  }
}

Answer 1

You could first use the [] string operator to get the substrings instead of substring and drop the intermediate variables. For example in the case for length == 10:

"${isbn[0]}-${isbn[1..6]}-${isbn[7..8]}-${isbn[9]}"

Now, there is a bit of repetition there. You can get instead first get all the isbn segments and then .join them with '-':

[isbn[0], isbn[1..6], isbn[7..8], isbn[9]].join('-')

And, even further, instead of referencing isbn every time, you can make a list of the ranges you want to get and then get them all the same time using collect:

[0, 1..6, 7..8, 9].collect { isbn[it] }.join('-')

If you're going for code golfing, you can also do:

('-'+isbn)[1, 0, 2..7, 0, 8..9, 0, 10]

I'll leave it to you to figure out how that works, but i guess it's probably not a good idea to leave that on production code, unless you want to surprise future maintainers hehe.

---

Also, notice that the format when length == 13 is the same as for length == 10 but with a different prefix, you can then reuse the same function in that case. The whole function (with a couple of tests) would be:

/**
 * 10 digit - #-######-##-#
 * 13 digit - ###-#-######-##-#
 **/
def formatIsbn(isbn) {
    switch (isbn?.length()) {
        case 10: return [0, 1..6, 7..8, 9].collect { isbn[it] }.join('-')
        case 13: return isbn.take(3) + '-' + formatIsbn(isbn.drop(3))
        default: return isbn
    }
}

assert formatIsbn('abcdefghij') == 'a-bcdefg-hi-j'
assert formatIsbn('abcdefghijklm') == 'abc-d-efghij-kl-m'

---

Now, i think there are some bad smells in that code. Can isbn be null? At least to me, this doesn't look like a function that needs to bother about the nullity of its argument, or at least that's not clear by reading its name (it should be called something like formatIsbnOrNull instead if both ISBN strings and null values are accepted). If null values are not valid, then let it blow up with a NullPointerException when accessing isbn.length() so the caller know they have passed a wrong argument, instead of silently returning the same null.

The same goes for the return ISBN at the end. Is it expected for that function to receive a string that's neither 10 nor 13 characters long? If not, better throw new IllegalArgumentException() and let the caller know they have called it wrongly.

---

Finally, i'm not sure if this is the most "readable" solution. Another possible solution is having a string for the format, like '###-#-######-##-#' and then replace the #s by the isbn characters. I think it might be more self-documenting:

def formatIsbn(isbn) {
    def format = [
        10: '#-######-##-#',
        13: '###-#-######-##-#'
    ][isbn.length()]
    def n = 0
    format.replaceAll(/#/) { isbn[n++] }
}