When I pass an ArrayList's get method a variable, it's using the whole ArrayList

Question

For some reason, when I pass my ArrayList (which is a basic ArrayList from Java's Lang.*) an int variable, instead of taking the item at the index of the variable's int (eg: i = 0; arrayList.get(i); should be the first item in the ArrayList) - it calls the WHOLE list.

Here's the method in question:

public ZipCode findZip (int zip) {
    ZipCode aZip = new ZipCode(00000);
     //System.out.println(zips.get(0)); here gives 99501,ANCHORAGE,AK
     //System.out.println(zips.get(0).getZipCode()); here gives 99501
        for(int i = 0; i < zips.size(); i++) {
            if(zips.get(i).getZipCode() == zip)
                aZip = zips.get(i);
              //System.out.print(aZip); here gives the zip codes from the whole array
            else
                aZip = null;
        }
    return aZip;
    //Therefore, aZip is ALWAYS null. Even if it exists.
}

I've tried a bunch of troubleshooting to figure it out. So far, I've learned that the issue doesn't arise when I put a straight integer in (eg: arrayList.get(1); gets the second item as normal). There's no compiler error to show, unfortunately.

It always gives me null when it runs.

I need to use an int so I can increment it to run through the list, also unfortunate.

Larger Clip of Program: (Same as from my prior question)

import java.util.*;
import java.io.*;
import java.lang.Math;
public class ZipCodeDatabase {
   //Field
    private ArrayList<ZipCode> zips;

   //Constructor
   public ZipCodeDatabase () {
      zips = new ArrayList<ZipCode> (); 
   }

   //Mutator Method
   public void readZipCodeData(String filename) {
      Scanner inFS = null; 
      FileInputStream fileByteStream = null;
      try{
       // open the File and set delimiters
         fileByteStream = new FileInputStream(filename);
         inFS = new Scanner(fileByteStream);
         inFS.useDelimiter("[,\r\n]+");
       // continue while there is more data to read
         while(inFS.hasNext()) {
            //read in all input
            int aZip = inFS.nextInt();
            String aCity = inFS.next();
            String aState = inFS.next();
            double aLat = inFS.nextDouble();
            double aLon = inFS.nextDouble();
            //Create and add new zipcode
            ZipCode newZip = new ZipCode(aZip, aCity, aState, aLat, aLon);
            zips.add(newZip);
         }
         fileByteStream.close();
         // Could not find file
         }catch(FileNotFoundException error1) {
            System.out.println("Failed to read the data file: " + filename);
          // error while reading the file                      
         }catch(IOException error2) {
             System.out.println("Oops! Error related to: " + filename);
      }        
   }

   //Accessor Methods
   public ZipCode findZip (int zip) {
      ZipCode aZip = new ZipCode(00000);
      for(int i = 0; i < zips.size(); i++) {
          if(zips.get(i).getZipCode() == zip)
            aZip = zips.get(i);
         else
            aZip = null;
      }
      return aZip;
   }

Here's the getZipCode() method (it returns the zip code int, a part of a small collection of ints including latitude and longitude; of which I have an array):

public int getZipCode () {
       return zipCode;
}

What is zips? Can you show a complete example that reproduces the problem? — assylias
– assylias, Commented Nov 9, 2015 at 18:00
Why do you have else aZip = null? That will change aZip to refer to null even if a match was found earlier in the loop. I don't think you want that else statement at all. — Louis Wasserman
– Louis Wasserman, Commented Nov 9, 2015 at 18:04

HulkSmash · Accepted Answer · 2015-11-09 21:41:38Z

4

You don't stop looping when you find the zip code, so your next iteration through the loop will usually set your return value to null. Change your statement to this:

Edited for your professor:

 ZipCode aZip = null;
 for(int i = 0; i < zips.size() && aZip == null; i++) {

    if(zips.get(i).getZipCode() == zip) {
            aZip = zips.get(i);
    }
 }
 if (aZip == null) {
    aZip = new ZipCode(00000);  
    // return new ZipCode(00000); would be better
 }

 return aZip;

edited Nov 9, 2015 at 21:41

answered Nov 9, 2015 at 18:07

HulkSmash

3961 silver badge6 bronze badges

Sign up to request clarification or add additional context in comments.

8 Comments

N. Spock Over a year ago

I have to return a variable, and the prof gets pissed about returns within loops.

hagrawal7777 Over a year ago

Then just use break keyword after aZip = zips.get(i);. break means do not loop any further because I have got what I was looking for.

Andy Turner Over a year ago

Is it bad style to use 'return' to break a for loop in Java?

Andy Turner Over a year ago

You can also put the break implicitly into the for statement declaration: for(int i = 0; i < zips.size() && aZip.getZipCode() == 0; i++) {. But returning from inside a loop is fine in my book.

N. Spock Over a year ago

BUT. That gives me an idea.

|

fps · Accepted Answer · 2015-11-09 18:27:41Z

1

The problem is that you are not stoping the iteration when you find the target ZipCode, and are assigning null to your aZip variable in a subsequent iteration.

To solve it, you could use a condition to stop iterating:

ZipCode aZip = null;

for (int i = 0; (i < zips.size()) && (aZip == null); i++) {
    if (zips.get(i).getZipCode() == zip) {
        aZip = zips.get(i); // When you set this, aZip will no longer be == null
    }
}

answered Nov 9, 2015 at 18:27

fps

34.5k8 gold badges62 silver badges116 bronze badges

Comments

N. Spock · Accepted Answer · 2015-11-09 18:34:39Z

0

Thank you everyone!

While the other answers do work, here's a different solution that is giving what seems to be correct answers:

public ZipCode findZip (int zip) {
      ZipCode aZip = null;
      for(int i = 0; i < zips.size(); i++) {
          if(zips.get(i).getZipCode() == zip) {
            aZip = zips.get(i);
          }
      }
      return aZip;
   }

edited Nov 9, 2015 at 18:34

answered Nov 9, 2015 at 18:32

N. Spock

435 bronze badges

3 Comments

fps Over a year ago

That's my answer! :P Mine is optimized, though, because it stops iterating when the item is found.

Holger Over a year ago

Sure it does. It was you who said the else part was mandated by your prof.

N. Spock Over a year ago

The null was, so I didn't know another way to put it

Holger · Accepted Answer · 2015-11-09 18:39:18Z

Since you don’t stop the loop after finding a match, the next iteration’s result will overwrite your match, which is most probably a non-matching item, hence overwrite it with null. There are several options:

break out:

public ZipCode findZip (int zip) {
  ZipCode aZip = null;
  for(int i = 0; i < zips.size(); i++) {
      if(zips.get(i).getZipCode() == zip) {
        aZip = zips.get(i);
        break;
     }
     else
        aZip = null;
  }
  return aZip;
}

change the loop condition:

public ZipCode findZip (int zip) {
  ZipCode aZip = null;
  for(int i = 0; i < zips.size() && aZip==null; i++) {
      if(zips.get(i).getZipCode() == zip)
        aZip = zips.get(i);
     else
        aZip = null;
  }
  return aZip;
}

don’t overwrite:

public ZipCode findZip (int zip) {
  ZipCode aZip = null;
  for(int i = 0; i < zips.size(); i++) {
      if(zips.get(i).getZipCode() == zip)
        aZip = zips.get(i);
     else
        continue; // obsolete, but you said, your prof insist on it…
  }
  return aZip;
}

As a final note, the cleanest solution is to provide a correct equals method in you ZipCode class. If the ZipCode class has a proper equals implementation, you can implement the method as simple as:

public ZipCode findZip (int zip) {
  ZipCode aZip = new ZipCode(zip);
  if(zips.contains(aZip))
    return aZip;
  else
    return null;
}

or, even shorter:

public ZipCode findZip (int zip) {
  ZipCode aZip = new ZipCode(zip);
  return zips.contains(aZip)? aZip: null;
}

Collectives™ on Stack Overflow

When I pass an ArrayList's get method a variable, it's using the whole ArrayList

4 Answers 4

8 Comments

Comments

3 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

8 Comments

Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related