principlejavaMinor
Little part of the string to integer parser vs Clean Code
Viewed 0 times
theparserpartlittlecodestringintegerclean
Problem
I started reading a Clean Code book and some other articles on the Internet about OOD and design patterns and I'm a little confused. I've started to code a little string to integer parser which currently parses a first integer in string larger than 0 or returns -1.
Could you look and give me some idea if this is written correctly with general rules of good programming? I'm not sure about usage of
IntegerParser.java
IntegerSubstringer.java
```
package stringParser.integerParser;
public class IntegerSubstringer
{
IntegerFinder finder;
String text;
Index startIndex;
Index endIndex;
public IntegerSubstringer()
{
finder = new IntegerFinder();
}
public String cutFirstNumber( String text )
{
this.text = text;
setStartIndexAtTheFirstNumber();
iterateToTheEndOfTheNumberSeries();
if (startIndex.isInvalid()) return new String();
return makeSubstringAccordToIndexes();
}
private void iterateToTheEndOfTheNumberSeries()
{
setEndIndexAfterStartIndex();
while( endOfTextNotReached() && finder.letterIsANumber(text, endIndex) )
{
endIndex.
Could you look and give me some idea if this is written correctly with general rules of good programming? I'm not sure about usage of
Index instances in the IntegerSubstringer class.main methodpublic static void main (String Args[])
{
IntegerParser parser = new IntegerParser();
System.out.println( parser.getFirstInt("Blaola") );
}IntegerParser.java
package stringParser.integerParser;
public class IntegerParser
{
IntegerSubstringer substringer;
public IntegerParser()
{
substringer = new IntegerSubstringer();
}
public int getFirstPositiveInt(String textToParse)
{
String numberSubstring = substringer.cutFirstNumber(textToParse);
if(!numberSubstring.equals(""))
return parse(numberSubstring);
return -1;
}
private int parse(String substringContainingNumbersOnly)
{
return Integer.parseInt(substringContainingNumbersOnly);
}
}IntegerSubstringer.java
```
package stringParser.integerParser;
public class IntegerSubstringer
{
IntegerFinder finder;
String text;
Index startIndex;
Index endIndex;
public IntegerSubstringer()
{
finder = new IntegerFinder();
}
public String cutFirstNumber( String text )
{
this.text = text;
setStartIndexAtTheFirstNumber();
iterateToTheEndOfTheNumberSeries();
if (startIndex.isInvalid()) return new String();
return makeSubstringAccordToIndexes();
}
private void iterateToTheEndOfTheNumberSeries()
{
setEndIndexAfterStartIndex();
while( endOfTextNotReached() && finder.letterIsANumber(text, endIndex) )
{
endIndex.
Solution
Index = int
My first thought as I skimmed through your code was that you have a whole class that seems to be nothing more than a wrapper for an int. The only method of any significance is
KISS
I didn't read the rest of the code very thoroughly but it seems like a lot of code to solve a simple problem. I prefer the KISS methodology (keep it simple stupid) to complicated solutions. I'm sure if you tried to rewrite your code with simplicity as a goal, you could come up with something much smaller and simpler.
Here's an example of how you could simplify. You have this function:
Notice that all it does is call another function. You could remove this function and replace the call to it with the call to
In fact many of your functions are one liners. Instead of having these one line functions which are mostly called from one place only, you could just put the one line in the calling function and add a comment about what is happening (if it isn't clear already). Right now, it almost seems like every line of code is a function call to a new function.
My first thought as I skimmed through your code was that you have a whole class that seems to be nothing more than a wrapper for an int. The only method of any significance is
isInvalid() which could easily be moved to somewhere else or just inlined when needed. I think it would be best to remove this class and just use ints as indices.KISS
I didn't read the rest of the code very thoroughly but it seems like a lot of code to solve a simple problem. I prefer the KISS methodology (keep it simple stupid) to complicated solutions. I'm sure if you tried to rewrite your code with simplicity as a goal, you could come up with something much smaller and simpler.
Here's an example of how you could simplify. You have this function:
private int parse(String substringContainingNumbersOnly)
{
return Integer.parseInt(substringContainingNumbersOnly);
}Notice that all it does is call another function. You could remove this function and replace the call to it with the call to
Integer.parseInt directly. Now someone reading the code will know exactly what your code is doing when they read that line. With your current code, they would have to skip to the definition of parse to figure out what is happening, because the function name "parse" doesn't really explain what it does.In fact many of your functions are one liners. Instead of having these one line functions which are mostly called from one place only, you could just put the one line in the calling function and add a comment about what is happening (if it isn't clear already). Right now, it almost seems like every line of code is a function call to a new function.
Code Snippets
private int parse(String substringContainingNumbersOnly)
{
return Integer.parseInt(substringContainingNumbersOnly);
}Context
StackExchange Code Review Q#88732, answer score: 4
Revisions (0)
No revisions yet.