Calculating multiple car prices from a file - follow-up My application needs to read a file, which includes a list of cars from different factories, and calculate the prices (by this I mean mean add up their prices).

I suppose my solution is efficient.  Is there any better approach to it?

 String factories = "Benz,10000,BMW,3000,Benz,5,Toyota,500,BMW,50000,RR,100000,BMW,100000,Jaguar,100000,Benz,4,Next,100,Benz,3,Benz,2,Benz,1,Toyota,100000,Polo,10,Gall,1000";
        String[] fac = factories.split(",");
        List&lt;Company&gt; trn = new ArrayList();
        for(int i=0;i&lt;fac.length-1;i=i+2){
            trn.add(new Company(fac[i],fac[i+1]));
        }
        Map&lt;String, BigDecimal&gt; map = new HashMap();

        BigDecimal counter = new BigDecimal("0");
        BigDecimal target = new BigDecimal("10000");
        for (Company c : trn) {
            String name = c.getName();
            if (!map.containsKey(name)) {
                counter = new BigDecimal("0");
                for (Company com : trn) {
                    if (com.getName().equalsIgnoreCase(name)) {
                        counter = counter.add(com.getPrice());
                        if ((counter.compareTo(target)) == 1) {
                            map.put(name, counter);
                        }

                    }
                }

            }
        }
        Set&lt;String&gt; set = map.keySet();
        System.err.println("&gt;&gt;" + set.toString());


Output


[RR, Toyota, BMW, Jaguar, Benz]
List&lt;Company&gt; trn = new ArrayList();


You're missing the &lt;&gt; (or &lt;Company&gt; if you're using JDK6 or below) on the right, which means the right side is a raw type and not a generic. Your IDE should be configured to warn you about this.

In addition, what does trn mean? It's completely unclear even from context. You should almost never use abbreviations unless they're industry-standard, and maybe not even then.

Map&lt;String, BigDecimal&gt; map = new HashMap();


This is also a raw type. Its name also tells me nothing. What are you mapping? A better name would be mapTotalPriceByFactoryName, or just totalPriceByFactoryName.

BigDecimal counter = new BigDecimal("0");


BigDecimal has a constant for this, BigDecimal.ZERO. Use it here and below.

for (Company c : trn) {


Another unnecessary abbreviation - call the loop variable company.

counter = new BigDecimal("0");


Is this actually a counter? It seems like a summation, and should be called something like totalPrice accordingly.

if (com.getName().equalsIgnoreCase(name)) {
    counter = counter.add(com.getPrice());
    if ((counter.compareTo(target)) == 1) {
        map.put(name, counter);
    }
}


This code is unclear to me. Why equalsIgnoreCase instead of just equals - do we expect the input file to be inconsistent with itself? That seems like an important caveat to call out in comments.

The behavior of the counter is also very unclear. You only want to store factories with a total price of at least 10000? It would be simpler to do the summation in one step, then filter out the unwanted factories in a subsequent step.

Set&lt;String&gt; set = map.keySet();
System.err.println("&gt;&gt;" + set.toString());


Here you never refer back to the values stored in the map. If you're not going to refer back to them, you might as well just use a List.

As an example, here's how I would store the prices:

Map&lt;String, BigDecimal&gt; map = new HashMap&lt;&gt;();
for (Company company : companies) {
    String name = company.getName();
    if (!map.containsKey(name)) {
        map.put(name, BigDecimal.ZERO);
    }

    BigDecimal totalPrice = map.get(name).add(company.getPrice());
    map.put(name, totalPrice);
}


This becomes an O(N) algorithm instead of the O(N^2) algorithm you're using currently. You can then post-process the map to filter out the companies you aren't interested in.