Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm extracting the version number from a file like this one. It works, but I find it clumsy and longer than probably needed.

I'd also appreciate some sanity checks as long as they don't make the code noticeably longer (It's just a script, no full Java parser. Something similar to Guava's getOnlyElement would be nice).

version = {->
    def lines = file('src/main/org/h2/engine/Constants.java').readLines();
    def extract = {name ->
        lines.grep {it =~ /public static final int $name =/}[0].replaceFirst(/.*=\s*(\d+);.*/, '$1')
    }
    extract('VERSION_MAJOR') + '.' + extract('VERSION_MINOR') + '.' + extract('BUILD_ID')
}()
share|improve this question
add comment

1 Answer

up vote 1 down vote accepted

Since you are only looking for a single occurrence of the variable in the file you could match the whole file at once. For example like this:

version = {->
    def text = file('src/main/org/h2/engine/Constants.java').text
    def extract = {name ->
        nameValue = text.replaceAll(/(?s)^.*public\s+static\s+final\s+int\s+$name\s+=\s*(\d+);.*$/, '$1')
        text.length() == nameValue.length() ? "Cannot find name" : nameValue;
    }
    extract('VERSION_MAJOR') + '.' + extract('VERSION_MINOR') + '.' + extract('BUILD_ID')
}()

Instead of getting the lines this script reads the file to one single String, and then uses a regular expression to replace the while text with the variable of interest.

In order to do this, we first make sure that the . in the regexp will match newlines by adding (?s) to the pattern. We then match everything from the start (^.*) up until public static final int $name, capture the value with (\d+), and then match everything to the end of the file with .*$.

Although the regular expression is slightly more involved, this way makes it easier to check whether the replacement has worked and prevents a NullPointerException. We can now simply compare the length of the original text with the length of the result. If they are the same the replacement has not succeeded and we can return a custom error-message, otherwise we found the variable of interest.

share|improve this answer
    
Wouldn't file(…).text work? –  200_success Jun 19 at 17:31
    
@200_success Good call, changed it. –  EricBouwers Jun 19 at 17:33
    
Could you explain what /(?s)/ does? Also, surely there's a way to do a capturing match instead of a replaceAll? –  200_success Jun 19 at 17:39
    
I have added an explanation for ?s, don't know about a capturing match. Maybe someone else? –  EricBouwers Jun 19 at 17:48
    
Nice, but I disagree with one thing: I really don't wan't to prevent NPE. If anything goes wrong, it should blow. An error message is deemed to get undetected, an exception blows the build and that's strictly better than getting a nonsense. –  maaartinus Jun 19 at 18:17
show 1 more comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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