wood burning stoves 2.0*
The moose likes Beginning Java and the fly likes Code review for beginer Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of OCM Java EE 6 Enterprise Architect Exam Guide this week in the OCMJEA forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Code review for beginer" Watch "Code review for beginer" New topic
Author

Code review for beginer

shivam kataka
Greenhorn

Joined: Feb 09, 2012
Posts: 3
hi,

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
thanks.

tempLines are all lines. lineNo is the line number of main line

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38509
    
  23
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 system property 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.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38509
    
  23
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.
Jeff Verdegan
Bartender

Joined: Jan 03, 2004
Posts: 6109
    
    6

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.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38509
    
  23
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.
Darryl Burke
Bartender

Joined: May 03, 2008
Posts: 4531
    
    5

Campbell Ritchie wrote:Different rules usually apply when you have to break a single statement into several lines.

I've always used double the normal indent, and not just in Java ... is that the practice most coders follow?


luck, db
There are no new questions, but there may be new answers.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38509
    
  23
Not sure, but I think most people use double indenting.
 
Don't get me started about those stupid light bulbs.
 
subject: Code review for beginer