i have lines in text file with the below pattern. i want to extract lines which are which are less than 21 characters for each main line
This is line no 1 this line has more than 21 characters //main line has more than 21 characters
This is line no 2 //has less than 21 characters
This is line no 3 //has less than 21 characters
This is line no 4 //has less than 21 characters
This is line no 5 this line has more than 21 characters //main line has more than 21 characters
This is line no 6 //has less than 21 characters
This is line no 7 //has less than 21 characters
This is line no 8 //has less than 21 characters
I wrote the method, which is working fine, but i need code review for it as i feel it can be optimised/written better
tempLines are all lines. lineNo is the line number of main line
Some dangerous code there. Never write an empty catch; if there is an Exception you will not know about it.
Use StringBuilder rather than StringBuffer, even though the performance overhead is slight.
Don’t use literal \n or \r unless you are specifically told to. Find the systemproperty which represents line end; I think it is called "line.separator". Create a String constant with that value, called LINE_END or similar and use that instead. Of course, if you are not splitting, you won’t need that after all.
I suspect you hare doing something very dubious, adding Strings to a StringBuffer and later splitting them into a String. You ought to be adding the Strings to a List<String> and passing that List as an argument with the source Strings in.
Your getSublines() method is inappropriately named. It does not return sublines of anything.
I would suggest you don’t hard-code 21. Get the length to be tested passed to a method parameter.
You are not testing for lines with < 21 characters. You are testing for lines with > 21 characters. Try it with a 21 character line and see what happens. Serious logic error there.
You may get a line with 20 letters in counted as ≥ 21 chars, if any of those is a supplementary character in Unicode. If that is a possibility, you should use the code point count method of String rather than length.
Delete the nextLine local variable and use i + 1 instead. It will of course cause an ArrayIndexOutOfBoundsException when you get to the end of the array! And you ought not catch that.
You are not at all clear about your requirements for files which have several lines over 21 characters long. That method can only return the first array of lines.
It would be better to return a String or List<String> rather than joining all the lines together.
I don’t like the return in the middle of the loop. It looks as if you were not sure how to skip the first line. Can you guarantee the first line is longer than 21?
Your null check will never fail; I do not believe the split method can return an array containing null elements.
That’s probably enough to keep you busy for at least ten minutes.
Joined: Oct 13, 2005
Your indentation is also inconsistent; at least you are using spaces, not tabs, bu you are sometimes using 8 and sometimes 4. Always use 4.
Campbell Ritchie wrote:Your indentation is also inconsistent; at least you are using spaces, not tabs, bu you are sometimes using 8 and sometimes 4. Always use 4.
Whenever possible, I use 2.
Joined: Oct 13, 2005
In that case I shall instruct OP always to use 3
Good point. The actual number of spaces varies from convention to convention, but it should always be n × i spaces where n is spaces per level and i is number of indents. Different rules usually apply when you have to break a single statement into several lines.