6
\$\begingroup\$

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;
}
\$\endgroup\$
  • \$\begingroup\$ Have you done any performance test which shows that this is a performance bottleneck? 20k item isn't so much... \$\endgroup\$ – palacsint Oct 30 '11 at 18:29
  • \$\begingroup\$ 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 :) \$\endgroup\$ – Marcx Oct 30 '11 at 19:55
5
\$\begingroup\$

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.

\$\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.