2
\$\begingroup\$

I have an implementation of an edit text to show line numbers and also highlight the current line. I think it needs more optimization because when I scroll through the edit text with about 500 lines of code, it lags, yet when I comment out the code, the scrolling is smooth.

I need help optimizing this onDraw method in which I do everything above. The line numbers appear on the right with a custom separator e.g

  1|
...
 10|
..
100|
..
public class NumberedEditText extends AppCompatEditText {

  private Rect nRect;
  private Rect nRect_;
  private int nOffset;
  private Paint nPaint;
  private Paint nPaint_;
  private boolean nShow;
  private int nAlignment;
  private char nSeparator;
  private boolean nLineHlt;

  public NumberedEditText (Context context, AttributeSet attrs) {
    super (context, attrs);

    nRect = new Rect ();
    nRect_ = new Rect ();
    nPaint = new Paint ();
    nPaint_ = new Paint ();

    nAlignment = NUMBER_ALIGNMENT_LEFT_SEP_LEFT;
    nSeparator = '|';
    nLineHlt = true;
    nShow = true;
    nOffset = 2;

    nPaint_.setColor (Color.parseColor ("#222222"));

    nPaint.setColor (Color.LTGRAY);
    nPaint.setStyle (Paint.Style.FILL);
    nPaint.setTypeface (Typeface.MONOSPACE);
  }

  @Override
  public void onDraw (Canvas canvas) {
  // I use short variable names because i think
  // they are executed much faster.
    if (nShow) {
      nPaint.setTextSize (getTextSize () - nOffset);
    
      int bl = getBaseline ();
      int lc = getLineCount ();
      int lh = getLineHeight ();
    
      // i use an array here for efficient
      // random access to store the
      // positions where the line numbers
      // will be according to the number
      // of lines
      String lcs = Integer.toString (lc);
      int nl = lcs.length ();
      char[] ch = new char[nl + 1];
      int cl = ch.length;
      String s;
    
      // i fill the array with spaces
      // ready to be replaced with numbers
        for (int i = 0; i < nl; i++)
          ch [i] = 32;
       // set the last character to the separator
        ch [nl] = nSeparator;
        // now i loop thru all the numbers
        // replacing the characters in the array
        // (with spaces) with numbers
        // to form something like
        // '   1|'
        // '  10|'
        // ' 100|'
          for (int i = 1, j = cl - 2; i <= lc; i++, bl += lh) {
            if (i == 10 || i == 100 || 1 == 1000 || i == 10000 || i == 100000)
              j--;
            s = Integer.toString (i);
            if (j > -1)
              s.getChars (0, s.length (), ch, j);

            canvas.drawText (ch, 0, cl, nRect.left, bl, nPaint);
          }
          // now i set the padding and an
          // additional margin
      setPadding ((int) nPaint.measureText (lcs + "__"), getPaddingTop (), getPaddingRight (), getPaddingBottom ());
    }

    // this is to highlight the current line
    Layout l = getLayout ();
    if (nLineHlt && l != null) {
      int ln = l.getLineForOffset (getSelectionStart ());
      getLineBounds (ln, nRect_);
      canvas.drawRect (nRect_, nPaint_);
    }
    super.onDraw (canvas);
  }
}
```
\$\endgroup\$
1
\$\begingroup\$

// I use short variable names because i think
// they are executed much faster.

You are wrong. Execution speed does not depend on the length of identifiers anyhow.

// i use an array here for efficient
// random access to store the
// positions where the line numbers
// will be according to the number
// of lines

As usual the use of (primitive) arrays leads to procedural solution involving al lot of looping. Unnecessary looping is the most common performance issue.

   // now i loop thru all the numbers
   // replacing the characters in the array
   // (with spaces) with numbers

Instead of looping you should calculate what you need.

Because of your poor naming I'm not analyzing in detail what your code really does, but it looks like in mainly calculates the indentation needed. Given the number of the current line and the overall line count I would do it this way:

 // this could also be a member variable 
 // so that it does not need to be 
 // calculated each time the draw method runs
 int indentedNumberWith =
           String.valueOf(lineCount).length();

 String indentedLineNumber =
           String.format(
                         "% "+indentedNumberWith+"s",
              // note this ^ space
                         String.valueOf(lineNumber));

This should replace the loop with the if cascade...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.