This method is not inserting the objects into the ArrayList. Why?

Question

public void addClimb(String peakName, int climbTime){ 
  for(int i = 0; i < climbList.size()-1; i++){
    if(peakName.substring(0,1).compareTo(climbList.get(i).getName().substring(0,1)) <= 0){
             climbList.add(i, new ClimbInfo(peakName, climbTime));   
         } else {
             climbList.add(new ClimbInfo(peakName, climbTime));
         }
     }
 }

The Goal is to have it take in an peakName and climbTime, have it loop through the climbTime objects in climbList, check for when the first letter of the parameter comes before the first letter of the climbTime in the loop, and places it as soon as that happens, but im getting an out of bounds error when i enter in several ClimbInfos and try to print them. This method is not properly inserting the ClimbInfo into climbTime properly.

Could someone explain what I'm doing wrong?

why are you saying it's not properly inserting? do you get an error? if not , what do your array contents look like after this method? — SOfanatic, May 07 '13 at 02:04
I'm sorry, i meant to say that it's not inserting at all. Im getting an out of bounds exception, and the compiler seems to be telling me that my array size is 0. — Sam D20, May 07 '13 at 02:07
You should add the code that shows your array declaration, also the line that where the `out of bounds exception` is happening. — SOfanatic, May 07 '13 at 02:09

score 4 · Answer 1 · answered May 07 '13 at 02:07

Depending on initial conditions, your function will do one of two things:

if climbList is initially empty, or initially contains only one element, then the test i < climbList.size() - 1 will fail, so the function will return immediately, doing nothing.
if climbList initially contains more than one element, then the test i < climbList.size() - 1 will always succeed, because every pass through the loop will add an element to climbList and will increment i by one. So i < climbList.size() - 1 is true before the iteration, then it will be true after the iteration. So you have an infinite loop.

I don't think that either of these is what you want.

Actually, if climbList is initially empty... it will always stay empty! — Julián Chamalé, May 07 '13 at 02:10

Bruce Martin · Answer 2 · 2013-05-07T07:13:41.873

0

Basically you are looping through the whole list, adding a record each time through the loop. You probably want something like:

public void addClimb(String peakName, int climbTime){ 
  for(int i = 0; i < climbList.size(); i++){
    if(peakName.substring(0,1).compareTo(climbList.get(i).getName().substring(0,1)) >= 0){
       climbList.add(i, new ClimbInfo(peakName, climbTime));
       return;
    }
  }

  climbList.add(new ClimbInfo(peakName, climbTime));

 }

edited May 07 '13 at 07:13

answered May 07 '13 at 02:11

Bruce Martin

10,358
1
27
38

In addition, the termination expression of the for loop shouldn't have "-1" in it or needs to be changed to <=. – Peter Alfvin May 07 '13 at 02:37
Correct peter, should of looked closer – Bruce Martin May 07 '13 at 07:14

score 0 · Answer 3 · answered May 07 '13 at 02:13

You failed to explain what the problem is.

This method is not properly inserting the ClimbInfo into climbTime properly.

Doesn't tell me what happens. Runtime exception? Compile error? Or are you left with an empty collection at the end?

If you receive an exception, please include it along with a stack trace.

If we assume you have an empty collection, that can only mean one thing: the for loop condition is never reached. I can tell this for certain because the if statement has an else which inserts. Therefore, every iteration of the loop is guaranteed to insert. Thus, the loop must not be iterating.

I suspect it's the size checking.

for(int i = 0; i < climbList.size()-1; i++){

That logic is almost certainly wrong for two reasons:

If you start with an empty collection, then climbList.size() = 0, which means climbList.size() - 1 = -1. 0 is not less than -1, therefore your condition fails and the loop exits.
Assume you start with a non-empty list, then you will in fact insert. However, every iteration of the loop rechecks the size, yet within the loop, you are appending. Essentially, you are saying "for every element in climb list, add an element to climb list". That is going to run out of memory at some point, unless you start with an empty list.

score 0 · Answer 4 · edited May 23 '17 at 12:28

0

you should not modify the list while iterating it.

I assume that what you are trying to achieve here is custom sorting. For that you must implement Comparable interface in ClimbInfo and use Collections.sort(climbList).

This method is not inserting the objects into the ArrayList. Why?

4 Answers4