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 1 down vote favorite
1

I'm developing an Android app to view C-programs.I wanted to give a simple color scheme to the text which is stored in database,retrieved as string and then passed on to the textview.

The code I have written assigns green color to header file declarations and brackets,blue color is assigned to numbers,printf,scanf...red is assigned to datatypes such as int,char,float.

It is however very inefficient.Before applying this color scheme,my app was displaying the textview activity instantly.Now,depending on the length of the C-programs, it takes up to 4 to 5 seconds which is really poor performance.

what it does is,it takes one keyword at a time,then iterates the complete text of textview looking for that particular keyword only and changes its color,sets the text again. Thus,it traverses text of the entire textview 29 times as I have defined 29 keywords in String arrays( namely keywordsgreen,keywordsblue,keywordsred).

The activity's onCreate fuction contains the following code :

        textView  = (TextView) findViewById(R.id.textView1);
        textView.setText(programtext);
        textView.setBackgroundColor(0xFFE6E6E6);

        //The problem starts here

        String [] keywordsgreen={"#define","#include","stdio.h","conio.h","stdlib.h","math.h","graphics.h","string.h","malloc.h","time.h","{","}","(",")","<",">","&","while ","for "};
        for(String y:keywordsgreen)
        {
        fontcolor(y,0xff21610B);
        }

        String [] keywordsred={"%d","%f","%c","%s","int ","char ","float","typedef","struct ","void "};
        for(String y:keywordsred)
        {
        fontcolor(y,0xFFB40404);
        }

        String [] keywordsblue={"printf","scanf","\n","getch","0","1","2","3","4","5","6","7","8","9"};
        for(String y:keywordsblue)
        {
        fontcolor(y,0xFF00056f);
        }

The fontcolor fuction is as follows :

private void fontcolor(String text,int color) 
    {
        Spannable raw=new SpannableString(textView.getText());
        int index=TextUtils.indexOf(raw, text);
        while (index >= 0) 
        {
          raw.setSpan(new ForegroundColorSpan(color), index, index + text.length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
          index=TextUtils.indexOf(raw, text, index + text.length());
        }
        textView.setText(raw);
    }
share|improve this question
up vote 1 down vote

First of all, some side notes:

  1. In your "keywordsgreen" array, you're checking for very specific library names. But you also check for #include and #define. So what if I include stdlib.h? Your current code would search the text, find "#include", turn it green, then search again, find "stdlib" and turn that green. But the whole line might as well be green. Instead, just look for the "#" symbol at the absolute start of the line (it would take a little more logic, of course). That way, you eliminate most of the keywordsgreen array.

  2. Also, you're setting the text every single time you edit a single word. You don't need to do that. Just keep one SpannableString and pass that around, setting the textView to that all the way at the end of the function.

  3. I don't think this should be that slow. It's not a very good solution, but even for a 300-line source file, I can't imagine it would take 5 seconds. Put all that in a new thread and see what happens. Example:

    Thread colorTextThread = new Thread(new Runnable(){
public void run(){ 
  //Put all that searching code in here
}//end of run
});
colorTextThread.start();

You put it on a new thread because the UI thread is super slow. Remember, though, if you want to change the textView on a separate thread, you have to use the runOnUiThread() function.

I'll add more later if no one tells you how to make the code faster. I've got to go at the moment, sorry.

EDIT:

After looking at your code again, I can't see a way to make it faster. The most important things you can do is to do the search on the "programText" variable you're passing to textView. That variable holds all the information you need, so you should manipulate that, and then pass it to textView.setText(). That way, you never have to call getText(), which means you can do all of your text processing on a separate thread. If it runs slow even on a separate thread, then yes, your code is very slow. Otherwise, problem solved for now.

share|improve this answer
up vote 0 down vote

I think the right way to do would be to have a map associating keywords to their color and them to go through your text, split it in tokens and for each token, apply the color if relevant.

This wouldn't be as good as a real syntax hilighter but it would probably work as good as your current code with better performances.

share|improve this answer
I'm a beginner in java.Could you please help me out with some sample code which shows how to do what you've written above ? – Umang Mathur Jul 4 at 20:29

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.