5
\$\begingroup\$

I worked starting from basic example code from an exercise in chapter 10 of Java: A Beginner's Guide, Sixth Edition, the code in the exercise was not very good and I wanted to improve upon it and make it my own, so this is what I came up with.

This reads the content of a text file, displays and reads titles/subjects, which lines are preceded by the # character, gets text input from the console, and displays information from searching the text file again for the user-input search.

I realize it's probably not very good, and I'm looking to improve my handling of I/O in general, please critique it thoroughly.

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.io.InputStreamReader;

/**
 * Help System using a text file as source.
 */
public class HelpSystem {
    private char titleLineMarker;
    private final String STOP_WORD = "stop";
    private final String HELP_SYSTEM_FILE_PATH = "C:/git/JavaExercises/src/chapter10/HelpSystem.txt";

    /**
     * Default constructor assumes character '#' is assumed to be the
     * line marker for titles/headers, according to standard Markdown syntax.
     */
    public HelpSystem() {
        titleLineMarker = '#';
        run();
    }

    /**
     * Alternate constructor with different line marker.
     * @param titleLineMarker the marker character that marks a line as subtitle.
     */
    public HelpSystem(char titleLineMarker) {
        this.titleLineMarker = titleLineMarker;
        run();
    }

    /**
     * Run the HelpSystem app.
     */
    private void run() {
        String topic;
        System.out.printf("Try the help system. Enter \"%s\" to end.", STOP_WORD);
        readTopics();
        do {
            topic = getSelection();
            if (!isStop(topic)) {
                if (!helpOn(topic)) {
                    System.out.printf("Topic not found: %s%n", topic);
                }
            }
        } while(!isStop(topic));
    }

    /**
     * Reads the help topics based on the character provided as a marker for lines containing a header.
     */
    private void readTopics() {
        System.out.println("Topics:");
        try(BufferedReader topicReader = new BufferedReader((new FileReader(HELP_SYSTEM_FILE_PATH)))) {
            int cursor;
            final char NOTHING = '\0';
            do {
                cursor = topicReader.read();
                if(cursor == titleLineMarker) {
                    System.out.println(" - " + topicReader.readLine().replace(titleLineMarker, NOTHING));
                }
            } while(cursor != -1);
        } catch(IOException exc) {
            exc.printStackTrace();
        }
    }

    /**
     * Search for a help topic entered by user.
     * @param what the help topic to search for
     * @return whether the topic is found
     */
    private boolean helpOn(String what) {
        int ch;
        String topic;
        String info;
        try (BufferedReader helpReader = new BufferedReader((new FileReader(HELP_SYSTEM_FILE_PATH)))) {
            do {
                ch = helpReader.read();
                if(ch == titleLineMarker) {
                    topic = helpReader.readLine();
                    if(what.compareTo(topic) == 0) { // found topic
                        do {
                            info = helpReader.readLine();
                            if(info != null && info.compareTo("") != 0) {
                                System.out.println(info);
                            }
                        } while(info != null && info.compareTo("") != 0);
                        return true;
                    }
                }
            } while(ch != -1);
        } catch (IOException exc) {
            exc.printStackTrace();
            return false;
        }
        // topic not found:
        return false;
    }

    /**
     * Attempt to find whether the topic the user entered exists.
     * @return whether the topic the user entered exists
     */
    private String getSelection() {
        String topic = "";
        BufferedReader inputReader = new BufferedReader(new InputStreamReader(System.in));
        System.out.print("Enter topic: ");
        try {
            topic = inputReader.readLine();
        } catch(IOException exc) {
            exc.printStackTrace();
        }
        return topic;
    }

    /**
     * Checks if the input matches the stop word.
     * @param input the input to search for
     * @return whether the input matches the stop word
     */
    private boolean isStop(String input) {
        return input.toLowerCase().equals(STOP_WORD);
    }
    // Test the class:
    public static void main(String[] args) {
        HelpSystem help = new HelpSystem();
    }
}

FYI, this is what it looks like when ran:

Try the help system. Enter "stop" to end.Topics:
 - if
 - while
Enter topic: if
if (condition) {
    // if statements
} else {
    // else statements
}
Enter topic: foo
Topic not found: foo
Enter topic: stop

Process finished with exit code 0

Based on text file HelpSystem.txt below - obviously could have more useful content, but this will do for now.

#if
if (condition) {
    // if statements
} else {
    // else statements
}

#while
while (condition) {
    // statements
}
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Interface

I don't think that instantiating a HelpSystem should have a side-effect of starting the interpreter loop. Either of these following designs would be better:

  • Instantiate, then run the loop separately

      HelpSystem help = new HelpSystem();
      help.run();
    
  • A static method

      HelpSystem.run();
    

It would be better for the no-argument constructor to chain to this('#'); instead of setting titleLineMarker itself. But why should the marker be configurable at all? Just settle on the file format an stick to it.

Note that anytime you have a special character, you should probably also have a mechanism for allowing that character to be treated literally. See RFC 821 Section 4.5.2 for an example of such an escaping mechanism.

Better yet, avoid inventing your own format, and use a standard format like or for which a parser already exists.

Printing the stack trace is not a very good way of handling an IOException. Such an exception should be considered fatal, not a situation to spew some text and try to continue. If you don't know what to do with an exception, just declare throws IOException to propagate it.

Parsing

Two of the private methods parse the help file. Assuming that the file is small, it should be better parse it into a data structure just once at the beginning.

try(BufferedReader topicReader = new BufferedReader((new FileReader(HELP_SYSTEM_FILE_PATH)))) has an extra pair of parentheses. It would be better written as

try (FileReader fr = new FileReader(HELP_SYSTEM_FILE_PATH);
     BufferedReader topicReader = new BufferedReader(fr);) {
    …
}

… both because it keeps the lines shorter and because it would close the FileReader properly if the FileReader constructor throws an exception.

Run loop

Your run() loop should be better structured to avoid the redundant isStop() checks.

private void run() {
    // readTopics();    <-- Perhaps better done in the constructor
    System.out.printf("Try the help system. Enter \"%s\" to end.", STOP_WORD);
    for (String topic; !isStop(topic = getSelection()); ) {
        if (!helpOn(topic)) {
            System.out.printf("Topic not found: %s%n", topic);
        }
    }
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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