Problems with program..

[spatel14]

Okay, so I'm writing a program that takes a .bin file and reads it in. Basically, the .bin file starts with 2 ints that represents a size of a map, for example, 7 and 7 (these represent pixels on a map). After the 2 ints, the rest of the file contains a whole lot of doubles that represent different elevations. The two ints are read in just normally, and then the remaining doubles are all fed into a 2D array. That's not my problem. I'm able to (at least I think I am) to do all this just fine. This is all done under the constructor. I have another method getMaximum which finds the maximum of the 2D array. Here's the code I have so far for both the constructor and the method:

Name of the program is Field, and this is the constructor

public Field(String path) throws IOException {

		if (path.substring(path.length()-4).equals(".bin")) {
			File file = new File(path);
			DataInputStream in = new DataInputStream(new FileInputStream(file));
			width = in.readInt();
			height = in.readInt();
			
			while (true) {
				try {
					elevation = new double[width][height];
					for (int i = 0; i < width; i++) {
						for (int j = 0; j < height; j++) {
							try{
								elevation[i][j] = in.readDouble();
							} catch(EOFException eof){
								break;
						
							}
						}
					}
					
				} catch (EOFException eof) {
					break;
				}
			}
		} else {
			System.out.println("File Incorrect, try again");
		}
	}

NOTE: And by the way, elevation is a static 2D array defined as a static variable to be used throughout the program as I will need access to it later, as well as width and height.

Name of method is getMaximum and here is the code

public double getMaximum() {
		if (elevation.length == 0 || elevation[0].length == 0) {
			return 0;
			} 
			double max = elevation[0][0];

			for (int i = 0; i < elevation.length; i++) {
				for (int j = 0; j < elevation[0].length; j++) {
					if (elevation[i][j] > max) {
						max = elevation[i][j];
					}
				}
			}
			return max;
	}

Now, when I try to create an instance of Field and run getMaximum with that instance, the program just keeps looping and looping and never stops running. I'm using an example file we were given to read in the numbers. So basically, the program should get all the numbers from the file, save the first two ints as height and width, and the rest as doubles in a 2D array. I can't figure out why the program doesn't work.. Again, my problem is that the program just keeps running and doesn't actually print anything out..

EDIT: Okay, so after some experimenting, I can say that the problem is not in the getMaximum method, so it must be in the constructor.. I can't see to pinpoint it though, can someone look it over for me to find the error?

EDIT2: Okay, so I think I solved it. In the while loop, it says while(true), so I think that the loop never goes false, so it just keeps going and going. So what I did was to create a boolean variable and set it to false before the while loop, and after the loop should be done, I set it to false. That seems to be working, but I'm going to do some more testing on it.

[m00nchile]

Yeah, while(true) loops can be a hassle. In your case, wouldn't it be easier to use a Scanner to read you file? Something like this shoud work well:

Scanner sc = new Scanner(someFile);
int i = 0;
while(sc.hasNextLine()) { //assume the first two ints are already read
  Scanner line = new Scanner(sc.nextLine());
  int j = 0;
  while(line.hasNextDouble()) {
    elevation[i][j] = line.nextDouble();
    j++;
  }
  i++;
}

[mangstadt]

Why do you need the while loop at all? When it's done filling the elevation array, you can just stop reading from the file. Also, you have one too many try/catch blocks..

[spatel14]

Yeah, while(true) loops can be a hassle. In your case, wouldn't it be easier to use a Scanner to read you file? Something like this shoud work well:

Scanner sc = new Scanner(someFile);
int i = 0;
while(sc.hasNextLine()) { //assume the first two ints are already read
  Scanner line = new Scanner(sc.nextLine());
  int j = 0;
  while(line.hasNextDouble()) {
    elevation[i][j] = line.nextDouble();
    j++;
  }
  i++;
}

Well, I tried that, but I was told (by my professor) that using DataInputStream is the only way (or at least most efficient way) to read .bin files.. I actually did try using Scanner and I got errors, so I went with DataInputStream

Why do you need the while loop at all? When it's done filling the elevation array, you can just stop reading from the file. Also, you have one too many try/catch blocks..

Well, I'm not sure on how to do that, this just seemed to be the easy way to do it. As for the second set of try/catch, I thought they would be needed for functionality, but I removed them and the program seems to be going strong, so they are removed for now.

[Norm]

DataInputStream is the only way

That is probably true. Data files have binary content. Scanner requires String content.

[Tolls]

Your inner break; is only breaking the inner for loop, so it will continue to go round the outer for loop, as well as the while loop. If you stuck a println in the inner catch you'd see that happening.

You either want a flag you can set, and alter the outer two loops (so "while(!eof){}", and adding "if (eof) break;"), or possibly label the while loop and break out of that? It's one of those rare instances I think the label might be justified.

[m00nchile]

A bit OT, but I think labels souldn't ever be justified. If you need this:

loop1 {
  loop2 {
    //I need to break out of both loops
  }
}

Instead of a label, I split the loops into 2 methods:

boolean innerLoop() {
  loop2 {
    if(condition) return false;
  }
  return true;
}

loop1 {
  if(innerLoop()) break;
}

[Tolls]

Fair point.
I've never personally used labels, but it was while writing about the flag that I thought a label might be the simpler option. Then again, breaking the whole functionality down a bit more, as you've done, is the neater one.

[mangstadt]

Well, I'm not sure on how to do that, this just seemed to be the easy way to do it. As for the second set of try/catch, I thought they would be needed for functionality, but I removed them and the program seems to be going strong, so they are removed for now.

In your main method, you just need to fill out the elevations array once. Do you need to fill it out twice, three times, four times? If not, then you have no reason to run the for loops again, so you don't need that while loop. :D

It would help to have one try/catch which surrounds the for loops, in case something's wrong with the .bin file you're using. Think about it like this: if you're using the wrong .bin file, do you want the program to continue reading from it when an error occurs? Having the try/catch surrounding the for loops will cause it to break out of the for loops when an error occurs.