4
\$\begingroup\$

I've implemented a program that spell checks a website. Here is the idea that I have in mind:

  • Scan all of the words in a web page into a string (using jsoup)
  • Filter out all of the HTML markup and code
  • Use a spell checking algorithm that reads from a dictionary.txt file and uses probability theory to offer suggestions

I would like to have my code reviewed and would greatly appreciate any input on how to make it more efficient or clean.

There is probably some bad practice followed as I'm new to programming so I apologize in advance if I'm doing something that is obviously wrong.

Some problems I've noticed about my code:

  1. It only accepts English words
  2. It prints out each suggestion in a new line, so large websites produce a messy output.

Here is the code:

Class 1 (used to call on the methods, basically a neat class to look good)

import java.io.*;

public class BulkSpellChecker extends ParseCleanCheck {

    public static void main(String[] args) throws IOException {
        System.out.println("Let's get started!");

        PageScanner(); // Scan the page and clean it first
        SpellChecker(); // Spell check the cleaned page

        System.out.println("Thanks for using the spell checker!");
    }}

Class 2

import java.io.*;
import java.util.*;

import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.safety.Whitelist;

public class ParseCleanCheck {

    static Hashtable<String, String> dictionary;// To store all the words of the
    // dictionary
    static boolean suggestWord;// To indicate whether the word is spelled
                                // correctly or not.

    static Scanner urlInput = new Scanner(System.in);
    public static String cleanString;
    public static String url = "";
    public static boolean correct = true;


    /**
     * PARSER METHOD
     */
    public static void PageScanner() throws IOException {
        System.out.println("Pick an english website to scan.");

        // This do-while loop allows the user to try again after a mistake
        do {
            try {
                System.out.println("Enter a URL, starting with http://");
                url = urlInput.nextLine();
                // This creates a document out of the HTML on the web page
                Document doc = Jsoup.connect(url).get();
                // This converts the document into a string to be cleaned
                String htmlToClean = doc.toString();
                cleanString = Jsoup.clean(htmlToClean, Whitelist.none());


                correct = false;
            } catch (Exception e) {
                System.out.println("Incorrect format for a URL. Please try again.");
            }
        } while (correct);
    }

    /**
     * SPELL CHECKER METHOD
     */
    public static void SpellChecker() throws IOException {
        dictionary = new Hashtable<String, String>();
        System.out.println("Searching for spelling errors ... ");

        try {
            // Read and store the words of the dictionary
            BufferedReader dictReader = new BufferedReader(new FileReader("dictionary.txt"));

            while (dictReader.ready()) {
                String dictInput = dictReader.readLine();
                String[] dict = dictInput.split("\\s"); // create an array of
                                                        // dictionary words

                for (int i = 0; i < dict.length; i++) {
                    // key and value are identical
                    dictionary.put(dict[i], dict[i]);
                }
            }
            dictReader.close();
            String user_text = "";

            // Initializing a spelling suggestion object based on probability
            SuggestSpelling suggest = new SuggestSpelling("wordprobabilityDatabase.txt");

            // get user input for correction
            {

                user_text = cleanString;
                String[] words = user_text.split(" ");
                 Set<String> wordSet = new HashSet<>();

                int error = 0;

                for (String word : words) {
                    if(!wordSet.contains(word)) {
                        checkWord(word);

                        suggestWord = true;
                        String outputWord = checkWord(word);

                        if (suggestWord) {
                            System.out.println("Suggestions for " + word + " are:  " + suggest.correct(outputWord) + "\n");
                            error++;
                        }
                    }

                        wordSet.add(word);
                    }

                if (error == 0) {
                    System.out.println("No mistakes found");
                }
            }

        } catch (IOException e) {
            e.printStackTrace();
            System.exit(-1);
        }
    }

    /**
     * METHOD TO SPELL CHECK THE WORDS IN A STRING. IS USED IN SPELL CHECKER
     * METHOD THROUGH THE "WORD" STRING
     */

    public static String checkWord(String wordToCheck) {
        String wordCheck, unpunctWord;
        String word = wordToCheck.toLowerCase();

        // if word is found in dictionary then it is spelled correctly, so
        // return as it is.

        if ((wordCheck = (String) dictionary.get(word)) != null) {
            suggestWord = false; // no need to ask for suggestion for a correct
                                    // word.
            return wordCheck;
        }

        // Removing punctuation at end of word and giving it a shot ("." or "."
        // or "?!")
        int length = word.length();

        // Checking for the beginning of quotes(example: "she )
        if (length > 1 && word.substring(0, 1).equals("\"")) {
            unpunctWord = word.substring(1, length);

            if ((wordCheck = (String) dictionary.get(unpunctWord)) != null) {
                suggestWord = false; // no need to ask for suggestion for a
                                        // correct word.
                return wordCheck;
            } else // not found
                return unpunctWord; // removing the punctuations and returning
        }

        // Checking if "." or ",",etc.. at the end is the problem(example: book.
        // when book is present in the dictionary).
        if (word.substring(length - 1).equals(".") 
                || word.substring(length - 1).equals(",")
                || word.substring(length - 1).equals("!") 
                || word.substring(length - 1).equals(";")
                || word.substring(length - 1).equals(":")) {
            unpunctWord = word.substring(0, length - 1);

            if ((wordCheck = (String) dictionary.get(unpunctWord)) != null) {
                suggestWord = false; // no need to ask for suggestion for a
                                        // correct word.
                return wordCheck;
            } else {
                return unpunctWord; // removing the punctuation and returning it
                                    // clean
            }
        }

        // Checking for (!,\,",etc) ... in the problem (example: watch!" when
        // watch is present in the dictionary)

        if (length > 2 && (word.substring(length - 2).equals(",\"") 
                || word.substring(length - 2).equals(".\"")
                || word.substring(length - 2).equals("?\"") 
                || word.substring(length - 2).equals("!\""))) {
            unpunctWord = word.substring(0, length - 2);

            if ((wordCheck = (String) dictionary.get(unpunctWord)) != null) {
                suggestWord = false; // no need to ask for suggestion for a
                                        // correct word.
                return wordCheck;
            } else // not found
                return unpunctWord; // removing the inflections and returning
        }

        // If after all of these checks a word could not be corrected, return as
        // a misspelled word.
        return word;
    }
}

Class 3

import java.io.*;
import java.util.*;
import java.util.regex.*;

class SuggestSpelling {

    private final HashMap<String, Integer> DataBaseWords = new HashMap<String, Integer>();

    /**
     * Method that reads the dictionary and checks for probability through word
     * occurrences
     */
    public SuggestSpelling(String file) throws IOException {
        try {
            BufferedReader in = new BufferedReader(new FileReader(file));
            Pattern p = Pattern.compile("\\w+");
            // Reading the dictionary and updating the probabilistic values
            // accordingly
            for (String temp = ""; temp != null; temp = in.readLine()) {
                Matcher m = p.matcher(temp.toLowerCase());
                while (m.find()) {
                    // This will serve as an indicator to probability of a word
                    DataBaseWords.put((temp = m.group()),
                            DataBaseWords.containsKey(temp) ? DataBaseWords.get(temp) + 1 : 1);
                }
            }
            in.close();
        } catch (IOException e) {
            System.out.println("Uh-Oh Exception occured!");
            e.printStackTrace();
        }
    }

    /**
     * 
     * Method that returns an array containing all possible corrections to the
     * word passed.
     * 
     */
    private final ArrayList<String> edits(String word) {
        ArrayList<String> result = new ArrayList<String>();

        for (int i = 0; i < word.length(); ++i) {
            result.add(word.substring(0, i) + word.substring(i + 1));
        }
        for (int i = 0; i < word.length() - 1; ++i) {
            result.add(word.substring(0, i) + word.substring(i + 1, i + 2) + word.substring(i, i + 1)
                    + word.substring(i + 2));
        }
        for (int i = 0; i < word.length(); ++i) {
            for (char c = 'a'; c <= 'z'; ++c) {
                result.add(word.substring(0, i) + String.valueOf(c) + word.substring(i + 1));
            }
        }
        for (int i = 0; i <= word.length(); ++i) {
            for (char c = 'a'; c <= 'z'; ++c) {
                result.add(word.substring(0, i) + String.valueOf(c) + word.substring(i));
            }
        }
        return result;
    }

    /**
     * 
     * Method that compares input to dictionary words and returns words that are
     * correct while checking for corrections on the others
     * 
     */
    public final String correct(String word) {
        if (DataBaseWords.containsKey(word)) {
            return word; // this is a perfectly safe word.
        }
        ArrayList<String> list_edits = edits(word);
        HashMap<Integer, String> candidates = new HashMap<Integer, String>();

        for (String s : list_edits) // Iterating through the list of all
                                    // possible corrections to the word.
        {
            if (DataBaseWords.containsKey(s)) {
                candidates.put(DataBaseWords.get(s), s);
            }
        }
        // In the first stage of error correction, any of the possible
        // corrections from the list_edits are found in our word database
        // DataBaseWords
        // then we return the one verified correction with maximum probability.
        if (candidates.size() > 0) {
            return candidates.get(Collections.max(candidates.keySet()));
        }
        // In the second stage we apply the first stage method on the possible
        // collections of the list_edits.By the second stage statistics

        for (String s : list_edits) {
            for (String w : edits(s)) {
                if (DataBaseWords.containsKey(w)) {
                    candidates.put(DataBaseWords.get(w), w);
                }
            }
        }

        return candidates.size() > 0 ? candidates.get(Collections.max(candidates.keySet()))
                : "Sorry but no possible corrections found!";
    }

    public static void main(String[] args) throws IOException {
        if (args.length > 0) {
            System.out.println((new SuggestSpelling("wordprobabilityDatabase.txt")).correct(args[0]));
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

Style convention

As you have used correctly in some places, Java's default convention is to use camelCase for method names, so having PageScanner() and SpellChecker() is mildly jarring to look at.

You also use a mixture of PascalCase, snake_case and camelCase for variable names, and the default convention for non-static final variables is to use camelCase as well. Standardization is highly recommended here.

Inheritance

BulkSpellChecker extends ParseCleanCheck

This looks slightly odd, especially when BulkSpellChecker is just, in your words, 'a neat class to look good'. If all you are doing is to implement public static void main(String[] args), you can do it in the underlying classes too. Extending a class only to implement static methods is a poor demonstration of inheritance.

Implementation vs interface

All Almost all your Collection classes are declared by their implementations (ArrayList) instead of their interfaces (List). It is usually recommended to use interfaces so that users of those variables only need to know they are dealing with a List. This allows for substitution too, e.g. during testing or to thread-safe implementations if required.

In addition, since Java 7, you can rely on the generic type infererence to shorten the declaration as such:

// ArrayList<String> result = new ArrayList<String>();
List<String> result = new ArrayList<>();

Hashtable

In 2017, Hashtable is pretty much a relic class and you are highly encouraged to switch over to HashMap or ConcurrentHashMap, as you already did elsewhere.

System.exit

A hard System.exit(int) is usually not recommended, especially when it does not sit inside the main() method (it's at least easier to follow there). If you really do encounter a serious error, propagate the exception to the callers until you can handle it safely, e.g. by prompting the user to re-enter.

Variables naming

// This do-while loop allows the user to try again after a mistake
do {
    try {
        System.out.println("Enter a URL, starting with http://");
        // ...
        correct = false;
    } catch (Exception e) {
        System.out.println("Incorrect format for a URL. Please try again.");
    }
} while (correct);

Reading correct here is quite misleading as it sounds like you will loop when the processing inside the code block is correct. One suggestion is to invert the meaning so that it better reflects what is being done here:

boolean isDone = false;
while (!isDone) {
    try {
        System.out.println("Enter a URL, starting with http://");
        // ...
        isDone = true;
    } catch (Exception e) {
        System.out.println("Incorrect format for a URL. Please try again.");
    }
}

Actually, you can also eliminate the flag entirely, and by packaging the method as one that actually returns a usable output instead of assigning static variables, you will get something like:

public static String getHtmlOutput(Scanner input) {
    System.out.println("Pick an english website to scan.");
    while (true) {
        try {
            System.out.println("Enter a URL, starting with http://");
            Document doc = Jsoup.connect(input.nextLine()).get();
            return Jsoup.clean(doc.toString(), Whitelist.none());
        } catch (Exception e) {
            System.out.println("Incorrect format for a URL. Please try again.");
        }
    } while (correct);
}

This showcases how the Scanner object reading from System.in (or potentially other sources) is taken in as the input, and returns the output of Jsoup.clean(String, Whitelist).

try-with-resources

Since Java 7, you can rely on try-with-resources for safe and efficient handling of the underlying IO resources. For example:

public static void main(String[] args) {
    String htmlOutput;
    try (Scanner scanner = new Scanner(System.in)) {
        htmlOutput = getHtmlOutput(scanner);
    }
    // ... do something with htmlOutput
}

Map methods

Since Java 8, there's Map.merge(K, V, BiFunction) to simplify the following kind of operations:

// words.put((temp = m.group()), words.containsKey(temp) ? words.get(temp) + 1 : 1);
words.merge(m.group(), 1, Integer::sum);
  • Use m.group() as the key.
  • Use 1 as the default value.
  • If the entry exists, apply the BiFunction Integer.sum(int, int) (as a method reference) to sum the existing value and the incoming value 1.
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Wow, thank you! I wasn't expecting any answers to be this helpful. You've definitely left me with a lot of reading up to do about the new java topics. I'll try my best to implement what you've suggested. \$\endgroup\$ – ary Apr 2 '17 at 8:12
3
\$\begingroup\$

Structure

You want to make your classes as reusable as possible. Extracting all the words from a website is something that is actually useful in other contexts, as is spellchecking.

Each class should have only one responsibility. You separated these rather well in text (scan, filter, spellcheck), but not in your code.

Your ParseCleanCheck class for example:

  • reads user input
  • echoes output
  • reads a website
  • parses HTML
  • spellchecks

That's just too much.

First of all, extract all the reading of input and echoing of output elsewhere (either just to main, or in dedicated Input and Output classes, with methods such as readURL, printError, printMessage, printWordSuggestion, etc).

Secondly, try to think about what each class does, and what it should do. Your names are a very good indication that something is not quite right here. ParseCleanCheck isn't so much a class name, as it is three verbs in a row. Actually, class names should be nouns, while methods should be verbs (a bit simplified: classes represent a thing, while methods represent an action on the thing). You have it the other way around.

Extract the code that reads and parses HTML into it's own class, for example SimpleHTMLParser with the method getWords. You can then have another classs which does the spellcheck.

You should then create instances of the class when using it (eg SimpleHTMLParser htmlParser = new SimpleHTMLParser('https://...'). This will avoid all the static keywords you use, which is strongly discouraged.

Finally, you should remove your extends. Extension essentially means that one thing is the same type as another, but more specific. You are just using it to add a main method.

tl;dr: You should look into the basics of object oriented programming

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you so much. I can see how implementing separate classes would be a lot neater than an jumbled up triverb class. I'll try to spend some time learning about the concept of OOP so that I can avoid making these mistakes in the future. \$\endgroup\$ – ary Apr 2 '17 at 8:20

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.