A little integer parser vs clean code

I was said to post this as a new question. I improved my code from:

Little part of the string to integer parser vs Clean Code

However I think that it break SRP as it parses, iterates through string and checks if letters are numeric. Could someone involved in OOP say what does he think of it? I agree that wrapping Index in class was a mistake, but it lost modularity in my opinion. I also see duplication in two IntegerParser's methods however I'm not sure how to remove it. In the end I think this is much less open for adding new functionalities.

Main.java

public static void main(String[] Args)
{
        IntegerParser intparser = new IntegerParser();
        System.out.println(intparser.getFirstNumber( "lolo543" ));
}

IntegerParser.java

package intparser;

public class IntegerParser
{
    private String parsingText;
    private int startIndex;
    private int lastIndex;

    public int getFirstNumber(  String textToParse )
    {
        parsingText = textToParse;
        startIndex = getIndexOfFirstNumericChain(0);
        lastIndex = getIndexOfLastNumberInChain(startIndex);
        if ( startIndex == -1 )
            return -1;
        if ( lastIndex == -1 )
            lastIndex = parsingText.length();
        return Integer.parseInt( parsingText.substring(startIndex, lastIndex) );
    }

    private int getIndexOfFirstNumericChain( int index )
    {
        if ( index = '0' && letter <= '9' )
            return true;
        return false;
    }   
}

Prologue

To be frank, this is a sub-optimal problem to be applying object-oriented programming concepts, for two reasons that I can think of:

• It is only a trivial step to extract the first subsequence of digits from a String, and

• Java has a number of built-in methods to do that.

Object-oriented programming concepts are more applicable to richer classes ("models") with appropriately-scoped actions, in layman terms. For example, you may want to drive(Direction.FORWARD) a Car, make a Duck quack() or process(Message) messages using a MessageProcessor.

Learning the APIs, or the Alternative Approach

Before any attempts to write new code, you often need to understand what are some of the existing solutions to the problem at hand. This implies you need to be reasonably familiar with the provided features/APIs from your programming language of choice, and subsequently third-party libraries.

Using Java as an example, unless it's for self-learning purposes, I will not expect to see new and extremely similar variations of a List implementation for every codebase or project I come across: I simply expect the use of the ArrayList objects to do that.

So, conveniently, what are the some of the other available solutions for this? Regex, Pattern and Matcher come into play:

public static int getFirstNumber(CharSequence input) {
    Matcher matcher = Pattern.compile("[0-9]+").matcher(input);
    return matcher.find() ? Integer.parseInt(matcher.group()) : -1;
}

What is being done here:

• The pattern "[0-9]+" (or "\\d+") represents a subsequence of digits we want to find, which must at least be a digit long.

• The surrounding parentheses specifies that we want to reference our matches using grouping: Following clarification by @Boris the spider.

• The Pattern object here represents the regex pattern, and certain interactions it can have with String (or more accurately, CharSequence) inputs.

• We then pass our input to matcher() to create our Matcher object.

• The Matcher object here represents the results of applying the regex pattern on the input.

• We then 'ask' by matcher.find() to see if we have matched any subsequences.

• If we do, we can get our first match by matcher.group(), else we return -1 as you have used as such.

• edit: As suggested by @ Machinarius, you may consider throwing an Exception to indicate that no subsequences were found.

• You want new features? Sure! Matcher has other methods that lets you process on the input iteratively. For example, you can use it to iterate till the last subsequence, you can combine all hits into a larger number, etc. It is not as restrictive as you might think.

Hopefully, this shows you why you do not have to keep writing new code every time.

Reinventing the wheel?

Now, suppose you do not wish to rely on regex for this problem. What are some of the improvements you can use for your solution?

• You should make your methods static, as it is generally not a recommended approach to keep creating wrapper objects for 'one-off' operations.

• Instead of storing textToParse as a class-level field, pass it between your methods.

• You should only look for the last index if startIndex is not -1. It's nice that you do a bit of input validation here, but that becomes redundant if you structure your method calls correctly.

• The return statement from numberAt(int) should just be return letter >= '0' && letter






• As far as my understanding goes, there isn't a clean, short and simple way to de-duplicate the two loops you are running across the getIndexOf... methods. Well, I can think of a slightly more fluent way using Java 8's IntStream and IntPredicate but I'll defer that suggestion for the time being, partly also because I don't think it's a good fit.






• Consistent bracing, this can be a pet peeve of some developers... Java's recommended way of using braces for methods (and if statements) is actually in my alternative solution above.




Unit testing

Last but not least, one of my more favorable topics here... It is not good enough to just display the output of your method. You need to perform assertions on the output to ensure your method is working as it should. This is where unit testing frameworks like JUnit, TestNG and Hamcrest matchers come into play to facilitate the writing of unit testing code. As a quick illustration, you should have a something similar to the snippet below that can be run as a unit test. Somewhat conveniently, both JUnit and TestNG are using their respective @Test annotations, so it wouldn't make a difference here. assertThat() and equalTo()` are from the Hamcrest library.

public class IntegerParserTest {
    @Test
    public void doTest() {
        assertThat(Integer.valueOf(IntegerParser.getFirstNumber("abc123xyz456")), 
            equalTo(Integer.valueOf(123)));
    }
}

StackExchange Code Review Q#88850, answer score: 12