Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I wrote a java code which divides the line through the words of the text so that each substring of no more than MaxWidth. It exelent working, but much slow. Help to optimize! Here's the code:

Pattern pattern = Pattern.compile("(.{1," + symbols + "}(\\b|\\s))"); // symbols - MaxWidth
Pattern pattern2 = Pattern.compile("\\s.*");

while ((line = in.readLine()) != null) {  // reading file by lines                  
    String dopLine = "";
    if (pattern2.matcher(line).matches()) {
        // If a line begins with a space, it is the beginning of a paragraph and need to add \ n
        if(!tempDopLine.equals("")) { // tempDopLine - Some part of the previous line, which is not full screen
            tempStringBuffer.append(tempDopLine);
            tempStringBuffer.append("\n");
            lineCounter++;
            if (lineCounter == lines) {
                addPage(tempStringBuffer); // create page
                tempStringBuffer = new StringBuilder("");
                lineCounter = 0;
                numberOfPages++;
            }

        }
        tempDopLine = "";
        dopLine = line;
    } else {   
        dopLine = tempDopLine + " " + line; // if this line
    }
    Matcher matcher = pattern.matcher(dopLine); // divide a string into a substrings

    HashMap<Integer, String> temp = new HashMap<Integer, String>();
    int i = 0;
    while (matcher.find()) {
        temp.put(i, matcher.group());
        i++;
    }
    for(i = 0; i < temp.size(); i++) {
        if (i<(temp.size()-1)) {
            String tempL = temp.get(i);

            tempStringBuffer.append(tempL);

            tempStringBuffer.append("\n");
            lineCounter++;
            if (lineCounter == lines) {
                addPage(tempStringBuffer);
                tempStringBuffer = new StringBuilder("");
                lineCounter = 0;
                numberOfPages++;
            }

        } else {
            tempDopLine = temp.get(i); // The last part of the string remember to display it along with the next line
        }
    }                       
}

How you can optimize this code? Please help and sorry for my poor English.

share|improve this question

1 Answer

up vote 2 down vote accepted

Few thought,

You do a while loop ( while( matcher.find() ) to find all matchs, then you do a for loop to deal with it. I think that can be done in only one loop.

in you last for loop, you could remove the first if :

   for(i = 0; i < (temp.size() -1); i++) {

     String tempL = temp.get(i);

     tempStringBuffer.append(tempL);

     tempStringBuffer.append("\n");
     lineCounter++;
     if (lineCounter == lines) {
            addPage(tempStringBuffer);
            tempStringBuffer = new StringBuilder("");
            lineCounter = 0;
            numberOfPages++;
     }

   } // End for loop

   tempDopLine = temp.get(temp.size()-1); 

}

Other suggestion the HashMap temp got be only a array, because right now your code do a lot of autoboxing ( from int to Integer) add and to retrieve information from your HashMap.

Edit: Here a quick example to merge your two loops:

String tempL = matcher.group();
while (matcher.find()) {

   tempStringBuffer.append(tempL);

   tempStringBuffer.append("\n");
   lineCounter++;
   if (lineCounter == lines) {
       addPage(tempStringBuffer);
       tempStringBuffer = new StringBuilder("");
       lineCounter = 0;
       numberOfPages++;
    }
    tempL =  matcher.group();


} 

tempDopLine = tempL;
share|improve this answer
To combine the two loop I need to get the number of elements found in the Matcher. How to do it? In my code - last match must writing to tempDopLine – Anton Feb 13 '12 at 18:36
Thank you very mych. I using this, but it did not give a large increase in performance. It seems to me that such a low rate because of the regular expressions. They can be something like speed? – Anton Feb 15 '12 at 7:15

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.