-2

I am making an app for android devices. There is a function in my app which has 2 "for loops" which iterate 3200 times each, accessing a 53 KB .txt file(which contains 3200 lines) and compare a string with each line, one line per iteration. The "for loops" also contain BufferedReader(), InputStreamReader(), InputStream() and StringTokenizer(). So it takes around 8 seconds for it to process that function, when I run the app on the emulator. This is inacceptable. How can I reduce the time required to, like, half a second, or max. 1 second? Thanks! edit:Here's is the part of my program with the 2 for loops:

else if(a==2){
        String z="";
        try{
            InputStream is = getAssets().open("USCOUNTIES.txt");
            InputStreamReader iz=new InputStreamReader(is);
            BufferedReader bis = new BufferedReader(iz);

            int v=0;

            v=count("USCOUNTIES.txt");//counts number of lines in the .txt file
        //finding no. of counties to be displayed
            int counter=0;
            String pos;
            pos=Integer.toString(position);
            try{
            for(int i=0;i<v;i++){
                z=bis.readLine();
                //int x=pos.length();
                boolean a;
                //using stringtokenizer
                StringTokenizer st = new StringTokenizer(z, ","); 
                String substring;
                substring=(String) st.nextElement();
                a=substring.equals(pos);
                if(a==true){

                    counter=counter+1;

                }
            }}catch(Exception e){e.printStackTrace();}
            String array1[]=new String[counter];

            try{
                InputStream ig = getAssets().open("USCOUNTIES.txt");
                InputStreamReader ia=new InputStreamReader(ig);
                BufferedReader bos = new BufferedReader(ia);
            int j=0;
            for(int i=0;i<v;i++){
                z=bos.readLine();
                String[] split = z.split(",");
                if(split[0].equals(pos)){
                    array1[j]=split[1];
                    j=j+1;
                }

            }}
            catch(Exception e){e.printStackTrace();}
ankit rawat
  • 401
  • 2
  • 9
  • 22
  • 1
    Honestly, 8 seconds to read 3200 files with 3200 lines each line by line is pretty good. – Raghav Sood Mar 16 '13 at 21:04
  • It is a single 3200 lined file. And it is iterated 3200 times. Any suggestion you might have? I do this to put some of the file's data in a list. – ankit rawat Mar 16 '13 at 21:08
  • 7
    Why are you re-reading the same file **3200 times**? – CommonsWare Mar 16 '13 at 21:08
  • Um, if its a single file why would you load it 3200 times? Just load it once outside you loop – Raghav Sood Mar 16 '13 at 21:09
  • Instead of using for loops, you could create a function that utilizes procedural recursion; that may cut down your overhead. – Thomas Anthony Mar 16 '13 at 21:09
  • 1
    what's the structure of your file? Seeing that you iterate the exact same amount of lines in your file, I suspect you can just iterate once and parse everything into a Map. – stealthjong Mar 16 '13 at 21:11
  • It is impossible to answer unless we see some code. – Don Roby Mar 16 '13 at 21:13
  • what is the purpose of compare, equality? if so, use a set. then u can find it n logn (instead of n^2) even if not, depending on your compare reason,you can stil use a proper data structure. posting your code would help. – yigit Mar 16 '13 at 21:16
  • @DonRoby I put in the code in an edit above – ankit rawat Mar 16 '13 at 21:24
  • The key point is that it's crazy to read the file inside the loops. Read it into something like a HashMap and lookup inside the loop. – Simon Mar 16 '13 at 21:27
  • @ChristiaandeJong: Structure is:0,1>Autauga;/n 0,2>Baldwin;/n 0,3>Barbour;/n goes on for 3200 lines. – ankit rawat Mar 16 '13 at 21:28
  • @kape123 I am new to android and serious java programming :) – ankit rawat Mar 16 '13 at 21:29
  • @Simon has good recommendation - just read your file into Hashmap and then play with that. Although I think you would benefit from posting your full function on http://codereview.stackexchange.com/ while saying what is that you want to do - I am sure the other part of function could also use optimizing. – nikib3ro Mar 16 '13 at 21:33
  • Here is the whole code on codereview.stackexchange.com: http://codereview.stackexchange.com/questions/23995/optimization-of-android-code-suggestions-iteration-based – ankit rawat Mar 16 '13 at 21:59

1 Answers1

1

If I were you, I'd parse everything only one time, and then do with it whatever you want.

This piece of code does just that, including parsing of the Integers (I suspect you need these as values, not as Strings):

public void read() throws IOException {
    InputStream is = getAssets().open("USCOUNTIES.txt");
    InputStreamReader iz=new InputStreamReader(is);
    BufferedReader bis = new BufferedReader(iz);
    String line = "";
    String firstNumber = "";
    String secondNumber = "";
    String countyName = "";
    StringTokenizer st = null;
    HashMap<Pair, String> map = new HashMap<>();
    while((line = bis.readLine()) != null) {
        st = new StringTokenizer(line, ",");
        firstNumber = (String) st.nextElement();
        st = new StringTokenizer((String)st.nextElement(), ">");
        secondNumber = (String) st.nextElement();
        countyName = ((String) st.nextElement());
        countyName = countyName.substring(0, countyName.length()-1);
        int num1 = Integer.parseInt(firstNumber);
        int num2 = Integer.parseInt(secondNumber);
        map.put(new Pair(num1, num2), countyName);
    }
}

class Pair {
    int num1, num2;
    Pair(int num1, int num2) {
        this.num1 = num1;
        this.num2 = num2;
    }

    public boolean equals(Object other) {
        if (other instanceof Pair) {
            Pair np = (Pair) other;
            return this.num1 == np.num1 && this.num2 == np.num2;
        }
        return false;
    }

    public int hashCode() {
        return (Integer.valueOf(num1).hashCode() >> 13) ^ Integer.valueOf(num2).hashCode();
    };
}

Now you can simply retrieve every countyName with this line:

String s = map.get(new Pair(1,69));

And that returns Aleutians East

I hope that gets you started.

EDIT

This piece of code uses a 2D SparseArray (much like an HashMap<Integer, Object>). With this, everything gets sorted by the first number.

public class Reader {
    private String firstNumber = "";
    private String secondNumber = "";
    private String countyName = "";
    private StringTokenizer stringTokenizer = null;
    private SparseArray<SparseArray<String>> sparseArray = new SparseArray<SparseArray<String>>();
    private SparseArray<String> temporarySparseArray = null;

    public void readFromIS() throws IOException {
        InputStream is = getAssets().open("USCOUNTIES.txt");
        InputStreamReader iz=new InputStreamReader(is);
        BufferedReader bis = new BufferedReader(iz);
        String line = null;
        while((line = bis.readLine()) != null) {
            readLine(line);
        }
    }

    public void readFromList() {
        String[] strings = {
                "0,1>Autauga;",
                "0,2>Baldwin;",
                "0,3>Barbour;",
                "1,69>Aleutians East;",     
                "1,68>Aleutians West;"
        };
        for (String line : strings) {
            readLine(line);
        }
    }

    private void readLine(String line) {
        stringTokenizer = new StringTokenizer(line, ",");
        firstNumber = (String) stringTokenizer.nextElement();
        stringTokenizer = new StringTokenizer((String)stringTokenizer.nextElement(), ">");
        secondNumber = (String) stringTokenizer.nextElement();
        countyName = ((String) stringTokenizer.nextElement());
        countyName = countyName.substring(0, countyName.length()-1);
        int num1 = Integer.parseInt(firstNumber);
        int num2 = Integer.parseInt(secondNumber);
        if (sparseArray.get(num1) == null) {
            sparseArray.put(num1, new SparseArray<String>());
        }
        temporarySparseArray = sparseArray.get(num1);
        temporarySparseArray.put(num2, countyName);
        sparseArray.put(num1, temporarySparseArray);
        temporarySparseArray = null;
    }

    public void test() {
        readFromList();
        String s = sparseArray.get(0).get(2);
        SparseArray sa = sparseArray.get(0);
        System.out.println(sa.size()); //should be 3
        System.out.println(s); // should be Baldwin
    }
}

And to retrieve all counties that start with num1, say, 0, you just use:

SparseArray<String> startingWithZero = sparseArray.get(0);

FYI: A SparseArray is a HashMap for integers, so not everything has to be autoboxed (from Integer to int, as you can't put primitive types in a HashMap).

EDIT2 you print the address of the 1D sparseArray.

public void printEverythingStartingWithZero() {
    SparseArray<String> subSparseArray = sparseArray.get(0); //You first need a 1D sparseArray
    int key = 0;
    for(int i = 0; i < subSparseArray.size(); i++) {
       key = subSparseArray.keyAt(i);
       String county = subSparseArray.get(key); //county is the String in place (0,key)
       System.out.println(county);
    }
}

You need to retrieve the 1D sparseArray first, with the leading zero.

stealthjong
  • 10,858
  • 13
  • 45
  • 84