4
\$\begingroup\$

I'm trying to get my code to conform to java formatting convention, I think I've spaced and indented the right way, however, your peer review would be much appreciated.

/**
* @author Nad Deb
* Date: 10/10/2020
* This program is used to play two words games
* It will work on input from the keyboard to workout suffix/infix/prefix from words
* derived from a dictionary text file.
*/

package wordgames;

//importing classes for method reading files, CLI input and, exception handling

import java.io.File;
import java.util.Scanner;
import java.io.FileNotFoundException;

public class WordGames {
/**
* This method is used to declare and initialize a constant class variable 
* dictionary. create scanner object class, declare string and int variables also
*/


private static final File FILEREADER = new File("DICTIONARY.txt");
private static String[] wordCollect;
private static int wordCounter = 0;
public static Scanner keyboardInput = new Scanner(System.in);

/**
* This main method is used to check for file exists with exception handling. 
* dictionary. create scanner object class, declare string and int variables also
*/

public static void main(String[] args) throws FileNotFoundException {
    //method for checking if file exists
    if (FILEREADER.exists() == false) {
    System.out.println("File doesn't exist.  Exiting.");
    System.exit(0);
    }
    wordCollect = new String[100];
Scanner fileScanner = new Scanner(FILEREADER);
    while(fileScanner.hasNextLine()) {
        String line = fileScanner.nextLine();
        wordCollect[wordCounter] = line;
        wordCounter++;
        }
    getSelection();
}

/**
* This get selection method is used to handle input from keyboard against a
* list of options from the menu, which are linked to methods using switch
* case.
*/

static String getSelection() throws FileNotFoundException {
    System.out.println("Welcome to the Word Games program menu.");
    System.out.println("Select from one of the following options.");
    System.out.println("1. Substring problem.");
    System.out.println("2. Points problem.");
    System.out.println("3. Exit.");
    System.out.println("Enter your selections: ");
    String selection = keyboardInput.next();
    
    //case switch for handling menu option input
    switch(selection) {
        case "1": subStringProblem();
        break;
        case "2": pointsProblem();
        break;
        case "3":
            System.out.println("Good Bye!");
            System.exit(0);
        break;
        default:
            System.out.println("Invalid option.  Try again.");
            getSelection();
    }
    return null;
}    
  
/**
* This substring method is used to concatenate *fix to end of string temp
* which is a string variable input from reading from lines in file via 
* wordCollect variable and checks if wordCollect starts with keyboard input
* substring.
*/
static void subStringProblem() throws FileNotFoundException {
    System.out.println("Substring problem.");
    System.out.println("Enter a Substring:");
    String subString = keyboardInput.next();
    String notFound = " - not found"; 
String infixFound = " - infix";
String prefixFound = " - prefix";
String suffixFound = " - suffix"; 
    for(int i = 0; i < wordCounter; i++) {
        String temp = wordCollect[i];
    
        boolean found = false;
                
        if(wordCollect[i].startsWith(subString)) {
            found = true;
            temp = temp + prefixFound;
        }
        if(wordCollect[i].endsWith(subString)) {
            found = true;
            temp = temp + suffixFound;
        }
        if(wordCollect[i].contains(subString)) {
            found = true;
            temp = temp + infixFound;
        }
        if(!found) {
            System.out.printf(" " + wordCollect[i] + 
                                    notFound  + "\n");
        }
                    else {
            System.out.printf(" " + temp + "\n");
        }   
}
    getSelection();
}

 /**
* This points problem method is used to read file lines from scanner input.
* switch case will check characters= (c) position in word string which is 
* input from nextLine, int l is the word length, for statement checks
* the characters as it goes along the world.  
*/
private static void pointsProblem() throws FileNotFoundException   {
    System.out.println("Points problem.");
    Scanner input = new Scanner(FILEREADER);  
    while (input.hasNext()) {
    String word = input.nextLine();
    int l = word.length();
    int point = 0;
    for (int x = 0; x < 0; x++) {  //checks the letters as it increments
        char c = word.charAt(x);   //checks the letter at position x         
        switch (c) {
            case 'a':
            case 'e':
            case 'i':
            case 'l':
            case 'n':
            case 'o':
            case 'r':
            case 's':
            case 't':
            case 'u':
                point += 1;
                break;
            case 'd':
            case 'g':
                point += 2;
                break;
            case 'b':
            case 'c':
            case 'm':
            case 'p':
                point += 3;
                break;
            case 'f':
            case 'h':
            case 'v':
            case 'w':
            case 'y':
                point += 4;
                break;

            case 'k':
                point += 5;
                break;
            case 'j':
            case 'x':    
                point += 8;
                break;
            case 'q':                        
            case 'z':
                point += 10;
                break;                       
        }
    }        
    System.out.println(word + "is worth " + point + " points." );
    }
    getSelection();                
}

}

\$\endgroup\$
  • 4
    \$\begingroup\$ Please make sure the preformatted code block correctly shows all of your code, indented the way you expect. Your class's final '}' is shown outside the code block. \$\endgroup\$ – Ralf Kleberhoff Oct 9 at 8:17
  • 8
    \$\begingroup\$ Regarding indentation: use your IDE's auto-indent or auto-format feature. Nobody does it by hand nowadays. \$\endgroup\$ – Ralf Kleberhoff Oct 9 at 8:18
  • \$\begingroup\$ WRT your title: please read codereview.stackexchange.com/help/how-to-ask . \$\endgroup\$ – BCdotWEB Oct 9 at 13:48
  • 1
    \$\begingroup\$ As you haven't edited your question yet, you're probably not interested in a review any more. \$\endgroup\$ – Ralf Kleberhoff Oct 12 at 12:56
  • \$\begingroup\$ Once a question has been answered, please do not edit the question, especially code that was mentioned in the review. \$\endgroup\$ – pacmaninbw Nov 14 at 14:01
3
\$\begingroup\$

You should use an automatic formatter like suggested in a comment, or an online one such as this https://www.tutorialspoint.com/online_java_formatter.htm or https://codebeautify.org/javaviewer

Some changes that we get are

Before

    switch(selection) {
        case "1": subStringProblem();
        break;
        case "2": pointsProblem();
        break;

After

    switch (selection) {
    case "1":
      subStringProblem();
      break;
    case "2":
      pointsProblem();
      break;

```
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you, I have already learnt to use the format function in Netbeans to format my code. \$\endgroup\$ – Nad Deb Nov 18 at 4:12
1
\$\begingroup\$
/**
 * @author Nad Deb
 * Date: 10/10/2020
 * This program is used to play two words games
 * It will work on input from the keyboard to workout suffix/infix/prefix 
   from words
 * derived from a dictionary text file.
 */

I'd skip such file headers altogether. There are package-level Javadoc, but ideally infomration about author, date etc. would be available through the version control.


package wordgames;

Ideally, the package names associate with the origin of the software, for example com.company.application or com.github.username.project.


//importing classes for method reading files, CLI input and, exception handling

Do not comment what you're doing, comment why and how you're doing it. Such comments are unnecessary as it tells me nothing beyond what I can read from the next three lines.


/**
* This method is used to declare and initialize a constant class variable 
* dictionary. create scanner object class, declare string and int variables also
*/

Same here, it is even misleading, as there is no method to be seen.


private static String[] wordCollect;
private static int wordCounter = 0;

You most likely want to use a List/ArrayList and call it simply words. Using a List would also allow you to remove the counter.


private static final File FILEREADER = new File("DICTIONARY.txt");
// ...
public static Scanner keyboardInput = new Scanner(System.in);

Ideally the file would be passed to an instance of your class, and you'd open the Scanner only in a scope as needed.


    if (FILEREADER.exists() == false) {
    System.out.println("File doesn't exist.  Exiting.");
    System.exit(0);
    }

System.exit is a a last resort to exiting a JVM. Returning from main would mean the JVM is shutting down, System.exit means that the JVM process is being terminated.


    wordCollect = new String[100];
Scanner fileScanner = new Scanner(FILEREADER);
    while(fileScanner.hasNextLine()) {
        String line = fileScanner.nextLine();
        wordCollect[wordCounter] = line;
        wordCounter++;
        }

Using a List here would remove most of the complexity, additionally, reading a file could be done through java.nio.Files.readAllLines, which returns a List of all lines. You also might need to think about the encoding that is used by the content.


        default:
            System.out.println("Invalid option.  Try again.");
            getSelection();

Instead of recursing, you should loop until a valid option is given.


static String getSelection() throws FileNotFoundException {

Why does this return String?


    for(int i = 0; i < wordCounter; i++) {
        String temp = wordCollect[i];

Could be replaced with a for-each loop when a List is being used.


        String temp = wordCollect[i];
    
        boolean found = false;
                
        if(wordCollect[i].startsWith(subString)) {
            found = true;
            temp = temp + prefixFound;
        }
        if(wordCollect[i].endsWith(subString)) {
            found = true;
            temp = temp + suffixFound;
        }
        if(wordCollect[i].contains(subString)) {
            found = true;
            temp = temp + infixFound;
        }
        if(!found) {
            System.out.printf(" " + wordCollect[i] + 
                                    notFound  + "\n");
        }
                    else {
            System.out.printf(" " + temp + "\n");
        }

As an improved exercise, encode the state in a class of its own. The constructor accepts a String as value, and there are three HasPrefix/HasSuffix/HasInfix methods which are returning whether these are present. Additionally, the toString could be overridden to have it return the wanted String representation.


for (int x = 0; x < 0; x++)

First, I'm a stubborn advocate that one should never user one-letter variable names, the only exception being dimensions.

Second, choosing x as a variable name is misleading, as most of the time one would think it would have something to do with a dimension.


for (int x = 0; x < 0; x++) {

That's not even working, at all, the condition is wrong.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.