Re: Rat In A Maze
From: Mark Haase (mehaase_at_earthlink.net)
Date: 08/24/04
- Next message: Tor Iver Wilhelmsen: "Re: Rat In A Maze"
- Previous message: Chris: "Rat In A Maze"
- In reply to: Chris: "Rat In A Maze"
- Next in thread: Tor Iver Wilhelmsen: "Re: Rat In A Maze"
- Reply: Tor Iver Wilhelmsen: "Re: Rat In A Maze"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Tue, 24 Aug 2004 08:31:05 -0400
Alright, I'll offer a brief code audit. If you're failing this class,
however, the last thing you need is somebody else doing your homework
for you.
Your first problem is just that: the problem.
You haven't stated the problem clearly. As I begin to look at your code,
I have no idea what its trying to do; I can only guess, because you
didn't say what the purpose was. If you don't have this fixed clearly in
your mind, you'll never write a good program to do it.
You also didn't say what made you think the program wasn't working as
intended. Does it compile? (No) Why not? What are the error messages? If
it did compile, what output did it give you and what output did you
expect? Why?
In article <459e0170.0408240251.9db8337@posting.google.com>,
chris24@hotmail.com (Chris) wrote:
>
> import java.io.*;
>
>
> public class FileIn{
>
> static int [] entrance={0,0};
> static int [] exit = {0,0};
> static int [] size = {0,0};
Bzzt. Bad idea. An int array in no way conveys the structure of a
coordinate. You should probably right your own class that includes an x
and y ordered pair
> static int spec = 0;
> static int [][] maze;
> static int [][] maze2;
I'm scanning the code from top to bottom, but it seems to me that
declaring two mazes is unnecessary.
> static String buffer= "";
> private static BufferedReader in = null;
> private static final int TRIED = 3;
> private static final int PATH = 7;
Some comments here are certainly in order. In fact it seems you haven't
commented at all. How on earth can you work in a group when nobody
explains what their portion of the code does?
*Whenever* you define named constants, you should explain what they're
for and how you picked those values. Reason being somebody else might
come along and say "3 & 7!? Who put that? It's supposed to be 4 &
8!"..not realizing the logic the original coder had used.
> public static int[] getInput(int [] input)
> {
>
> try{
> buffer=in.readLine();
> input[0]=Integer.parseInt(buffer.substring(0,1));
> input[1]=Integer.parseInt(buffer.substring(2));
>
> }
> catch(IOException e){
> System.out.println("Can't read input");
> }
> return (input);
> }//getInput
Without comments I have to presume that you've set this up to read your
special format. Make sure you understand how substring works, its not
intuitive.
> public static int getSpec()
> {
> int retval = 0;
> try{
> buffer=in.readLine();
> retval = Integer.parseInt(buffer);
> }
> catch(IOException e){
> System.out.println("can't read input");
> }
> return retval;
> }//getSpec
Again, I have no idea what getSpec() does...
How do you expect the TA to grade your assignment if its not commented
at all?
> public static int[][] makeMaze(int spec,int[] size)
> {
> if (spec==0)
> {
> int [][] retval = new int [size[0]][size[1]];
int[] is not the right type for size...<sigh>
Still, this is the first part of the code I've looked at so far where I
can at least hazard a guess what you're trying to do, so I'll take a
look and see how your algorithm works.
> try{
> for (int i=0;i!=size[0];i++)
Typically you'd not use != in a for loop evaluation condition. It's
fine, and works, but '<' is preferred, for esoteric reasons I won't go
into.
> {
> buffer= in.readLine();
> for (int j=0;j!=size[1];j++)
> {
> retval[i][j]=Integer.parseInt(buffer.substring(j,j+1));
You should really have included an example data file. How is it that you
know ahead of time how many digits each integer is? More to the point,
what does each int represent? Is it binary (wall/no wall) or do other
discrete values have meaning too?
> }//for
> }//for
> }
> catch(IOException e){
> System.out.println("Can't read input");
It would really help to make this error more informative. You use the
same error message every single place you have a caught IOException. If
you included something that described what function the error occurred
in, it would help you track down the error without even looking at the
stack trace. You could jump straight to the data file to see what was
wrong.
> }//catch
> return retval;
> }
> else
> {
>
> int test1=0;
> int test2=0;
> int [][] retval = new int[size[0]][size[1]];
> for(int i=0;i!=6;i++)
> {
> for(int j=0;j!=7;j++)
> {
> retval[i][j]=0;
> }//for
> }//for
You do realize you're not necessarily initializing all of the elements
of retval?
Furthermore, retval only exists in this narrow scope because you
declared it here. When you leave this scope you also lose retval. Shame,
since you try to return it at the end of the function. Looking back, I
see you made the same mistake the first time you declared retval as well.
> int count=0;
> while(buffer!=null)
> {
> try{
>
> buffer = in.readLine();
> if (buffer==null) break;
> test1=Integer.parseInt(buffer.substring(0,1));
> test2=Integer.parseInt(buffer.substring(2));
> test1-=1;
> test2-=1;
>
> retval[test1][test2]=1;
> }//try
> catch(IOException e){
> System.out.println("can't read input");
> }//catch
> }//while
> return retval;
> }//else
>
> }//makeMaze
What this last part does i can't guess.
> public static boolean traverse (int row, int column,int ex_row,
> int ex_col)
> {
> boolean done =false;
>
> if (valid(row,column))
> {
> maze[row][column] = TRIED;
>
> if (row==ex_row && column == ex_col) done=true;
> else
> {
> done = traverse (row+1,column,ex_row,ex_col);
> if(!done)
> done=traverse (row, column+1,ex_row,ex_col);
> if(!done)
> done=traverse (row-1,column,ex_row,ex_col);
> if(!done)
> done=traverse (row,column-1,ex_row,ex_col);
> }//else
>
> if (done)
> maze[row][column] = PATH;
> }//if
Ahhh!! So this is what "TRIED" and "PATH" are for...The values you
picked seem so arbitrary.
Anyway, you have the right idea here, trying to use recursion.
Unfortunately, thinking recursively is very hard to do.
Rather than pass ex_row and ex_col, you should just use the class
variables you defined earlier. This saves space on the stack.
This method is probably okay, however, if you did everything up to this
point correctly as well.
> return done;
> }
>
> private static boolean valid (int row, int column)
> {
> boolean result = false;
> if (row >=0&&row<maze.length&&column>=0&&column <maze[row].length)
> if (maze[row][column] == 0)
> result = true;
> return result;
> }//valid
You definitely need some spaces in that first if statement. And some
parentheses to clarify things. Assuming that the file format puts 0's
wherever the map is empty, this function is probably fine.
>
>
>
>
> public static void main(String[] args){
>
> try{
> in =
> new BufferedReader(
> new InputStreamReader(
> new FileInputStream("gp_input.txt")
> )
> );
> }//try
> catch(IOException e){
> System.out.println("Can't open "+args[0]);
> }//catch
>
Can't open args[0]? It doesn't even try..
>
>
>
> entrance = getInput(entrance);
> exit = getInput(exit);
> size = getInput(size);
> spec = getSpec();
>
> int row = entrance[0];
> int column = entrance[1];
>
> int ex_row = exit[0];
> int ex_col = exit[1];
>
> maze=makeMaze(spec,size);
> if (FileIn.traverse(row,column,ex_row,ex_col))
> System.out.println("The maze was successfully traversed!");
> else
> System.out.println("There is no possible path.");
>
>
> }//main
>
> }//FileIn
-- |\/| /| |2 |< mehaase(at)sas(dot)upenn(dot)edu
- Next message: Tor Iver Wilhelmsen: "Re: Rat In A Maze"
- Previous message: Chris: "Rat In A Maze"
- In reply to: Chris: "Rat In A Maze"
- Next in thread: Tor Iver Wilhelmsen: "Re: Rat In A Maze"
- Reply: Tor Iver Wilhelmsen: "Re: Rat In A Maze"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
