Getting Max value from Custom ArrayList

Question

I've wrote this function in Java:

public void calculateMax(Integer year){
    Double max = hoodDataList.get(0).getPercentage();
    if(year == 2006){
        for(Integer z = 0; z < hoodDataList.size(); z ++){

            if(hoodDataList.get(z).getYear() == 2006){
                if(hoodDataList.get(z).getPercentage() > max){
                    max = hoodDataList.get(z).getPercentage();
                }
            }
        }
    }
    Toast.makeText(getActivity(), String.valueOf(max),
            Toast.LENGTH_LONG).show();
}

Now if I'm correct it should calculate the maximum value of the Percentages and store it inside a variable called max but somehow it stops iterating after the second value

Tip: always use primitives unless you really need a wrapper object (such as Integer). Try to debug it or at least do System.out.println() on every element. From what I see everything seems to be fine. Maybe you have only two elements that are `if(hoodDataList.get(z).getYear() == 2006)`? — Cargeh, Apr 19 '17 at 16:30
From what I see this should work, atleast for the case your List is containing one or more items (otherwise a IOOBE is raised). Can you post your List of items? — Kevin Böckler, Apr 19 '17 at 16:31
This code works as you expect it should. What does your array contain? — lane.maxwell, Apr 19 '17 at 16:39

Ousmane D. · Accepted Answer · 2017-04-19T19:46:54.213

but somehow it stops iterating after the second value.

I've tested your code, with a custom type stored in hoodDataList however I can't seem to reproduce the error.

Alternatively, you can try this:

public void calculateMax(Integer year){
      if (year == 2006 && hoodDataList.stream().anyMatch(c -> c.getYear() == 2006))
      {
           double max = hoodDataList.stream().filter(c -> c.getYear() == 2006).
           max((c1, c2) -> Double.compare(c1.getPercentage(),c2.getPercentage())).
           get().getPercentage();
           Toast.makeText(getActivity(), String.valueOf(max),
           Toast.LENGTH_LONG).show();
      }else
      {
          //do something else
      }
}

side note - if you're going to use a for loop, don't use the wrapper type (Integer) as an indexer, just use the primitive int type.

Why waste 2x memory for that? What if he's got a big list? You can use a filter just like Jacob suggested above — Cargeh, Apr 19 '17 at 16:42

score -1 · Answer 2 · edited May 23 '17 at 12:25

-1

Maybe related to this? Not sure because I would expect that to cause no iterations, and max would always be set to hoodDataList.get(0).getPercentage(), but you say it stops after the second iteration.

edited May 23 '17 at 12:25

Community

1
1

answered Apr 19 '17 at 16:30

cerebro84

28
10

Jacob G. · Answer 3 · 2017-04-19T16:40:59.510

-1

With Java 8, you can use the following:

hoodDataList.stream().filter(i -> i.getYear() == 2006).mapToDouble(i -> i.getPercentage()).max().getAsDouble();

Note that hoodDataList should be a non-empty List, otherwise you can use OptionalDouble#orElseGet to provide a Supplier<Double> that returns a default value.

edited Apr 19 '17 at 16:40

answered Apr 19 '17 at 16:30

Jacob G.

28,856
5
62
116

I think the intention of this question is not to use an alternative approach but rather find the bug in the code above. – Kevin Böckler Apr 19 '17 at 16:34
1

He needs to check that the elements `getYear()` is 2006. I think filter would be a good idea, huh? – Cargeh Apr 19 '17 at 16:34

Getting Max value from Custom ArrayList

3 Answers3