I cannot even Java. Can anyone help?

[TheBook]

I have an assignment for my Java class where we are given a word search grid and a word list, and are supposed to write a program to find the words. I'm using string.indexOf() to search, but when I do, everything comes back as -1. Can someone help me figure out where I've gone wrong? I've attached the program folder so far and a text file with the actual code.

 

[David]

@TheBook
Okay, so you have a few different bad things going on here, but the most direct cause of the problem is with this:

public static int checkLR (int r, int c, String word, char[][] grid){
        String line = "";
        int retval = 0, x = 0;
        for (r = 0; r < 13; r++){
            line = "";
                for (c = 0; c < 13; c++){
                    line = line + grid[r][c];
                }
                System.out.println(line);
                retval = line.indexOf(word);
    }
        return retval;
 
    }

You're not returning retval until after the outer for loop has completed it's run, so unless the last row happened to have a match in it, you'll have a -1 returned to your caller.

------------------------------------------------------------
Edit: Now for some other tips to clean this all up a bit

There is really no need to pass r and c from main() to the checkLR() method since you aren't using their current values for anything. Usually when writing a for loop I'll declare the increment variable right on the spot for(int r = 0;...). The way you're doing it can work, but can also easily lead to bugs if you aren't careful.

The first time you call the checkLR() method from your main(), you are actually passing 13 and 13 for r and c from when you printed out the grid in main(). You never set them back to 0 until after the first iteration of the "word" loop, so if you had tried to use them somewhere other than the for loop first, you may have gotten unexpected results.

Also, you have int x=0 in there that isn't being used for anything.

 

[TheBook]

That's what I thought, but if I move the return statement into the loops, then I get a nasty error message. Could I set a boolean where if retval !=-1 it exits the loop and returns retval, or something along those lines?

 

[David]

@TheBook Yep, you'll want to check retval each time and return it as soon as it has a real value. You'll also need to leave a return statement outside of the loop where you currently have it for the case where there is no match going left to right. I'm guessing your error was because you only had the return inside the loop and weren't covering all of your bases. That way if retval comes back equal to -1, it will just mean the word wasn't found left to right, and you will eventually check the other directions.

 

[BobK]

One thing that should be mentioned other than @David's good advice is never, never, never use magic numbers such as the 13 that you use in your program (twice, and each instance represents something different). If you revisit the program six months from now you may not have a clue what the 13's mean. Worse, if the project gets turned over to another coder/maintainer they'll have even less of a clue what the 13 means. Yes you can read the code and figure out it reads the first 13 chars of the first 13 lines of the input array, but you shouldn't have to spend the time on that. And you could be tired/distracted/whatever and not get it right when you redecipher those card-coded numbers.

Most every language has functions to tell you what the size of an array/structure/whatever is. Use those instead.

 

[TheBook]

@TheBook

Edit: Now for some other tips to clean this all up a bit

There is really no need to pass r and c from main() to the checkLR() method since you aren't using their current values for anything. Usually when writing a for loop I'll declare the increment variable right on the spot for(int r = 0;...). The way you're doing it can work, but can also easily lead to bugs if you aren't careful.

The instructor prefers that we not declare variables at the beginning of a method, but I would rather do things this way.

The first time you call the checkLR() method from your main(), you are actually passing 13 and 13 for r and c from when you printed out the grid in main(). You never set them back to 0 until after the first iteration of the "word" loop, so if you had tried to use them somewhere other than the for loop first, you may have gotten unexpected results.

I moved the line setting r and c back to 0 to just after inFile.close();. Hopefully that should fix that.

Also, you have int x=0 in there that isn't being used for anything.

I think I was going to use that as a flag variable, but I figure a boolean would be better here (since there's only two options).

One thing that should be mentioned other than @David's good advice is never, never, never use magic numbers such as the 13 that you use in your program (twice, and each instance represents something different). If you revisit the program six months from now you may not have a clue what the 13's mean. Worse, if the project gets turned over to another coder/maintainer they'll have even less of a clue what the 13 means. Yes you can read the code and figure out it reads the first 13 chars of the first 13 lines of the input array, but you shouldn't have to spend the time on that. And you could be tired/distracted/whatever and not get it right when you redecipher those card-coded numbers.

Most every language has functions to tell you what the size of an array/structure/whatever is. Use those instead.

Java has a .length variable that I do usually use. Since I was having problems, though, I hard-coded them just so I knew they weren't causing the issue.

Here's what I have now:

    public static void main(String[] args) throws IOException {
        int r = 0, c = 0;
        int count = 0;
        char a = 'X';
        Scanner key = new Scanner(System.in);
        char [][] grid = new char [13] [13];
        System.out.println("Enter the name of the grid file:");
        Scanner inFile = new Scanner(new FileReader(key.nextLine()));
      
        //initialize array
        for (r = 0; r < grid.length; r++){
            count = 0; String x = inFile.nextLine();
            for (c = 0; c < grid.length; c++){
                a = x.charAt(count);
                grid [r][c] = a;
                count++;}}
      
        //display array
        for (r = 0; r < grid.length; r++){
            for (c = 0; c < grid.length; c++){
                System.out.print(grid[r][c] + " ");}
                System.out.println();}
        inFile.close();
        r = 0; c = 0;
      
        //get word list
        System.out.println("Enter the name of the word list file:");
        Scanner wordList = new Scanner(new FileReader(key.nextLine()));
        for (int w = 0; w <= 29; w++){
        if(wordList.hasNextLine()){
            String word = wordList.nextLine();//I know that everything up to here is good.
        int pos = checkLR(word, grid);
        if (pos == -1){System.out.println(word + " does not exist.");}
        else {System.out.println(word + " exists in row " + r +  " and column " + c);}      
        }
    }
    wordList.close();
}//end main

  
    public static int checkLR (String word, char[][] grid){
    String line = "";
            int retval = 0;
            boolean index = false;
            while (index = false){
            for (int r = 0; r < grid.length; r++){
                line = "";
              
                for (int c = 0; c < grid.length; c++){
                    line = line + grid[r][c];}
              
                retval = line.indexOf(word);
                if (retval != -1){
                        index = true;
                        return retval;
                }
                    else {index = false;}
            }
        }
    return retval;
    }
}

I must say, thank you both very much for your help so far.

 

[David]

@TheBook You're welcome. Glad we were able to help you so far.

So I think I see why you were passing r and c the way you were before. It didn't occur to me earlier because I was focused on checkLR more than main(). You wanted to have the last r and c values where the word was found so that you could print that index once you returned to main(). As your code is now it looks like it will just be printing that a word was found at row 0 column 0 no matter what since main() knows nothing about what went on in checkLR. I apologize for leading you in that direction.

You could put the code back to pass a reference to r and c BUT the better thing to do would be to leave the method signature as it is now and just print out the location/direction of a found word from within the method that is checking it instead of printing it in main(). Then just use the returned retval to decide whether to check in other directions if the word isn't found.

One more observation about your updated code though. The while loop and the index Boolean aren't really necessary. As soon as you get into the IF block and return the retval you'll have exited the checkLR() method and execution will continue in main(). There is no need to try to explicitly end the loop.

I'll be around most of the rest of the evening so if you need more clarification just ask. You're definitely on the right track.