current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite
2

I have this function that should merge two array of classes when id of array1 is equal to id of array2.

For simplicity I converted one array into an ArrayList so I can remove the occurrence merged into the other one..

I'm looking a way to improve it, also in cpu-time, because the two arrays can have more then 20000rows. The actual way, should take some time..

public Asset[] merge(Asset[] a2, ArrayList<countR> a)
{
        while (a.size()!=0){
            int i=0;
            Boolean b=false;
            while (i<a2.length && b==false)
            {
                if (a.get(0).id.equals(a2[i].id)){
                    a2[i].TotaleRegistrazioni = a.get(0).tot; 
                    b=true;
                }               
                i++;
            }
            a.remove(0);
        }
return a2;
}
share|improve this question
    
Have you done any performance test which shows that this is a performance bottleneck? 20k item isn't so much... –  palacsint Oct 30 '11 at 18:29
    
without looking at the time to scroll the entire array, I only would like to know if there is a better way to do this :) –  Marcx Oct 30 '11 at 19:55

1 Answer 1

up vote 4 down vote accepted

Staying with your original code you can change b to a simple break and you can change the inner while to a for which is easier to read:

public Asset[] merge2(final Asset[] firstArray, final ArrayList<CountR> secondList) {
    while (secondList.size() != 0) {
        for (int i = 0; i < firstArray.length; i++) {
            final CountR countR = secondList.get(0);
            final Asset asset = firstArray[i];
            if (countR.id.equals(asset.id)) {
                asset.TotaleRegistrazioni = countR.tot;
                break;
            }
        }
        secondList.remove(0);
    }
    return firstArray;
}

Searching in a HashMap should be faster than iterating through the whole array, so I'd do the following:

public Asset[] merge3(final Asset[] firstArray, final CountR[] secondArray) {
    final Map<Id, Asset> assetCache = new HashMap<Id, Asset>();
    for (final Asset asset : firstArray) {
        final Id id = asset.getId();
        assetCache.put(id, asset);
    }

    for (final CountR countR : secondArray) {
        final Id countRId = countR.getId();
        final Asset asset = assetCache.get(countRId);
        if (asset != null) {
            // I supposed that it's int
            final int total = countR.getTot();
            asset.setTotaleRegistrazioni(total); 
        }
    }

    return firstArray;
}

(I supposed that the Asset.id is unique in the array.) In this case you don't have to convert the second parameter to an array (just the first to a map). Maybe it worth playing with the initialCapacity and loadFactor parameters of the HashMap to avoid the resizing.

share|improve this answer

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.