19
\$\begingroup\$

This is a programming challenge I set for myself a while back to create an AI that starts with no knowledge of anything whatsoever, and learns as you talk to it. (It can learn stuff like your name, how to say hello, goodbye, etc.)

I tried to use the least amount of hard-coded responses as possible. I'm happy that it works, but I feel like the code behind it is very clumsy and... just not very good. The program is called Genesis. Tell me what you think!

Genesis.java

package genesis;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public final class Genesis {

public static final String name = "Genesis";
public static final String version = "1.0";

public static void main(String[] args) {
    try {
        new Genesis();
    } catch(IOException e) {
        printError(new PrintWriter(System.err), e);
    }
}

private final FileManager fm;
protected String last = null;

public Genesis() throws IOException {
    log("Initializing " + toString() + "...");
    log("Generating files...");
    fm = new FileManager(this);
    log(toString() + " started on " + System.getProperty("os.name"));
    start();
    stop();
}

public void stop() {
    stop(0);
}

public void stop(int error) {
    if(error == 0)
        log(toString() + " shut down successfully!");
    else
        log(toString() + " shut down with error code: " + error);
    if(fm != null)
        fm.close();
    System.exit(error);
}

public void start() {
    try(BufferedReader r = new BufferedReader(new InputStreamReader(System.in))) {
        System.out.print("You: ");
        String s = r.readLine();
        if(respond(s))
            start();
    } catch(Throwable t) {
        printError(t);
    }
}

public boolean respond(String s) { //decide how and what to respond, return if we should keep the program alive
    if(s.trim().equals(""))
        return true; //nothing to do, but keep the program alive
    String response = "";
    if(last == null) { //first message must always be a greeting
        boolean newg = true;
        for(String r : fm.getResponses(ResponseType.GREETING)) {
            if(transform(s).equalsIgnoreCase(transform(r)))
                newg = false; //if this is a greeting that we know about, we dont need to store it again
        }
        if(newg) //store a new greeting for use in another session (or this one)
            fm.setResponse(ResponseType.GREETING, removeEndPunc(s));
        //give a greeting back!
        System.out.println(response = (name + ": " + format(fm.getResponses(ResponseType.GREETING).get((int) (System.nanoTime() % fm.getResponses(ResponseType.GREETING).size())))));
    } else {
        boolean notg = true;
        for(String r : fm.getResponses(ResponseType.GREETING)) { //check if THE LAST MESSAGE is another greeting
            if(transform(last).equalsIgnoreCase(transform(r)))
                notg = false;
        }
        boolean notf = true;
        for(String r : fm.getResponses(ResponseType.FAREWELL)) { //check if they're saying a known farewell
            if(transform(s).equalsIgnoreCase(transform(r)))
                notf = false;
        }
        if((!notf || s.equalsIgnoreCase("exit")) && notg) { //if they're doing a farewell or saying "exit", and THE LAST MESSAGE is not a greeting
            boolean newf = true;
            for(String r : fm.getResponses(ResponseType.FAREWELL)) { //check if it's a new farewell
                if(transform(last).equalsIgnoreCase(transform(r)))
                    newf = false;
            }
            if(newf) //if it's new, store it for another session (or this one)
                fm.setResponse(ResponseType.FAREWELL, removeEndPunc(last));
            //say bye back
            System.out.println(response = (name + ": " + format(fm.getResponses(ResponseType.FAREWELL).get((int) (System.nanoTime() % fm.getResponses(ResponseType.FAREWELL).size())))));
            return false; //exit the loop
        }
    }

    boolean containsLaugh = false;
    for(String r : fm.getResponses(ResponseType.LAUGH)) { //are they laughing?
        if(s.matches(".*?\\b" + r + "\\b.*?"))
            containsLaugh = true;
    }
    boolean laughIfPossible = false;
    int laugh = 0;
    for(char c : s.toCharArray()) { //very bad laugh detection: >50% h's or l's
        if(c == 'h' || c == 'l')
            laugh++;
    }
    if(laugh > s.toCharArray().length / 2) { //if >50% h's or l's
        boolean newl = true;
        for(String r : fm.getResponses(ResponseType.LAUGH)) { //is this a laugh we know?
            if(transform(s).equalsIgnoreCase(transform(r)))
                newl = false;
        }
        if(newl) //if it's new, save it for later
            fm.setResponse(ResponseType.LAUGH, removeEndPunc(s));
        laughIfPossible = true; //if there's nothing else to say later, laugh
    }

    if(!containsLaugh) { //if super serious business mode is on
        String[] set = s.split("(\\s+is\\s+|\\'s\\s+)"); //regex: split for every "is" or 's ('s as in contraction for is)
        try { //if it's math, solve it
            System.out.println(response = (name + ": " + solve(transform(set[1]).trim())));
        } catch(Throwable t) { //it's not math
            String ek = transform(set[0]).toLowerCase(); //get the first part of the phrase
            if(ek.contains("what")) { //user is asking for information
                String k = transform(reversePerson(join(set, "is", 1).toLowerCase())); //get the object to look up
                for(String values : fm.getResponses(ResponseType.VALUE)) {
                    if(transform(values.split("§=§")[0]).trim().equalsIgnoreCase(k)) //if we know the information, tell the user
                        response = name + ": " + cap(k) + " is " + values.split("§=§")[1].trim() + punc();
                }
                if(!response.equals("")) //only respond if we have something useful to say
                    System.out.println(response);
            } else if(s.contains(" is ")) { //the user is telling us information
                String k = reversePerson(s.split(" is ")[0].toLowerCase().trim()); //the key to store
                String v = join(s.split(" is "), "is", 1).toLowerCase().trim(); //the value to store for the key
                fm.setResponse(ResponseType.VALUE, k + "§=§" + reversePerson(removeEndPunc(v))); //store the key and value
                System.out.println(response = (name + ": " + cap(k) + " is " + removeEndPunc(v) + punc())); //tell the user about our new information
            }
        }
    }
    if(response.trim().equals("") && (laughIfPossible || containsLaugh)) //if we have nothing else to say, but we can laugh, laugh
        System.out.println(response = (name + ": " + cap(fm.getResponses(ResponseType.LAUGH).get((int) (System.nanoTime() % fm.getResponses(ResponseType.LAUGH).size()))))); //say a random laugh
    fm.log("You: " + s); //log what the user said
    fm.log(name + ": " + (response.replace(name + ": ", ""))); //log our response, make sure to include the name even if we didn't earlier
    last = s; //set the new last message
    return true; //keep the program alive
}

private static String join(String[] set, String medium, int offset) { //join an array together with a specified string in between starting at a specified offset
    String s = set[offset];
    int i = 0;
    for(String part : set) {
        if(i > offset)
            s = s + " " + medium + " " + part;
        i++;
    }
    return s;
}

private static String reversePerson(String s) { //reverse between 1st and 3rd person, so Genesis makes sense
    return s.replaceAll("\\byour\\b", "§§m§§y§§").replaceAll("\\byou\\b", "§§m§§e§§").replaceAll("\\bme\\b", "you").replaceAll("\\bmy\\b", "your").replaceAll("\\byours\\b", "§§mi§§ne§§").replaceAll("\\bmine\\b", "yours").replace("§§", "").trim();
}

public static double solve(String c) { //solve math expressions
    Pattern p = Pattern.compile("(\\d+|\\d+\\.\\d+)\\s*(\\+|\\-|\\*|\\/|\\%|\\|)\\s*(\\d+|\\d+\\.\\d+).*"); //<number> <+-*/%|> <number>
    Matcher m = p.matcher(c);
    if(m.matches()) { //if this is a correct math expression
        Double d1 = Double.parseDouble(m.group(1));
        Double d2 = Double.parseDouble(m.group(3));
        while(c.contains("+") || c.contains("-") || c.contains("*") || c.contains("/") || c.contains("%") || c.contains("|")) { //checking for valid operations
            c = c.replaceAll("(\\d)\\.0(\\D)", "$1$2"); //replace all x.0 with just x (it looks better)
            m = p.matcher(c);
            if(!m.matches()) //this SHOULD match
                throw new ArithmeticException();
            switch(m.group(2)) { //check the operation symbol and do math
                default:
                    break;
                case "+":
                    c = c.replaceAll("[" + d1 + "]\\s*\\+\\s*[" + d2 + "]", (d1 + d2) + "");
                    break;
                case "-":
                    c = c.replaceAll("[" + d1 + "]\\s*\\-\\s*[" + d2 + "]", (d1 - d2) + "");
                    break;
                case "*":
                    c = c.replaceAll("[" + d1 + "]\\s*\\*\\s*[" + d2 + "]", (d1 * d2) + "");
                    break;
                case "/":
                    c = c.replaceAll("[" + d1 + "]\\s*\\/\\s*[" + d2 + "]", (d1 / d2) + "");
                    break;
                case "%":
                    c = c.replaceAll("[" + d1 + "]\\s*%\\s*[" + d2 + "]", (d1 % d2) + "");
                    break;
                case "|":
                    c = c.replaceAll("[" + d1 + "]\\s*\\|\\s*[" + d2 + "]", (Integer.parseInt((d1 + "").replace(".0", "")) | Integer.parseInt((d2 + "").replace(".0", ""))) + "");
                    break;
            }
        }
    } //else, maybe it's just a number (return the number)... or maybe it's not even math - if it is, return the parsed answer
    return Double.parseDouble(c);
}

private static String transform(String s) { //transform a string into something the program can read
    return s.toLowerCase().replace("?", "").replace(".", "").replace("!", "").replace(",", "").replace("_", "").replace("~", "").replace("`", "").replace("'", "").replace("\"", "").replace("\"", "").replace("\\", "").replace(":", "").replace(";", "").replace("the", " ").replace("teh", " ").replace("how do", "how can").replace("re", "").replace(" a ", " ").replace("is", "").replace("has", "").replace("get to", "go to").replaceAll("\\Bs\\b", "").replaceAll(" {2}?", "").trim();
}

private static String removeEndPunc(String s) {
    return s.replaceAll("[!\\.\\?]+$", ""); //remove all !'s .'s and ?'s from the end of a string
}

private static String format(String s) { //add random punctuation, and capitalize the first character
    return cap(s) + punc();
}

private static String cap(String s) { //capitalize the first letter of a given string
    String r = s.toUpperCase();
    if(r.length() > 1)
        r = s.replaceFirst(s.substring(0, 1), s.substring(0, 1).toUpperCase());
    return r;
}

private static char punc() { //return random punctuation
    switch((int) System.nanoTime() % 5) {
        default:
        case 0:
        case 1:
        case 2:
        case 3:
            return '.';
        case 4:
            return '!';
    }
}

public FileManager getFileManager() {
    return fm;
}

public void printError(Throwable t) {
    printError(System.err, t);
    if(fm != null)
        printError(fm.getLogStream(), t);
    stop(1);
}

private static void printError(Object output, Throwable t) {
    PrintWriterStream out;
    if(output instanceof PrintWriter)
        out = new PrintWriterStream((PrintWriter) output);
    else if(output instanceof PrintStream)
        out = new PrintWriterStream((PrintStream) output);
    else
        throw new IllegalArgumentException("Output must be of type PrintWriter or PrintStream");
    out.println();
    out.println("A fatal error occurred: " + t.toString());
    out.println();
    out.println("-----=[General Stack Trace]=-----");
    for(StackTraceElement s : t.getStackTrace())
        //print the throwable's stack trace
        out.println(s.getClassName() + "." + s.getMethodName() + "() on line " + s.getLineNumber());
    out.println("-----=[" + name + " Stack Trace]=-----");
    out.println();
    out.println("-----=[" + name + " Stack Trace]=-----");
    boolean fault = false;
    for(StackTraceElement s : t.getStackTrace()) { //filter out the stack trace for only Genesis
        if(s.getClassName().startsWith("genesis")) {
            out.println(s.getClassName() + "." + s.getMethodName() + "() on line " + s.getLineNumber());
            fault = true;
        }
    }
    if(!fault) //if it's not our fault, tell the user
        out.println("This doesn't look like a problem relating to " + name + ". Check the above general stack trace.");
    out.println("-----=[Genesis Stack Trace]=-----");
    out.println();
    out.println("-----=[Remote Stack Trace]=-----");
    fault = false;
    for(StackTraceElement s : t.getStackTrace()) { //filter out the stack trace for only outside Genesis
        if(!s.getClassName().startsWith("genesis")) {
            out.println(s.getClassName() + "." + s.getMethodName() + "() on line " + s.getLineNumber());
            fault = true;
        }
    }
    if(!fault) //if it's not their fault, tell the user
        out.println("This doesn't look like a problem with anything outside " + name + ". Check the above " + name + " stack trace.");
    out.println("-----=[Remote Stack Trace]=-----");
    out.println();
}

public void log(String message) {
    log(System.out, message);
}

public void log(PrintStream out, String message) {
    FileManager.log(out, message);
    if(fm != null)
        fm.log(message);
}

public String toString() {
    return name + " v" + version;
}

static class PrintWriterStream { //super hacky way of combining a PrintWriter and a PrintStream

    private PrintWriter w;
    private PrintStream s;

    PrintWriterStream(PrintWriter w) { //support for PrintWriter
        if(w == null)
            throw new NullPointerException();
        this.w = w;
    }

    PrintWriterStream(PrintStream s) { //support for PrintStream
        if(s == null)
            throw new NullPointerException();
        this.s = s;
    }

    void println() {
        if(w == null && s != null)
            s.println();
        else if(s == null && w != null)
            w.println();
        else
            throw new NullPointerException("No valid output");
    }

    void println(String x) {
        if(w == null && s != null)
            s.println(x);
        else if(s == null && w != null)
            w.println(x);
        else
            throw new NullPointerException("No valid output");
    }

    public void flush() {
        if(w == null && s != null)
            s.flush();
        else if(s == null && w != null)
            w.flush();
        else
            throw new NullPointerException("No valid output");
    }
}

}

FileManager.java

package genesis;

import genesis.Genesis.PrintWriterStream;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.HashMap;
import java.util.List;

public final class FileManager {

private final Genesis g;
public static final String folderPath = "Genesis/";
private final File folder = new File(folderPath);
private final File log = new File(folderPath + "log.txt");
private final File resp = new File(folderPath + "responses.txt");
private final PrintWriter lout;
private final PrintWriter rout;

public FileManager(Genesis g) throws IOException {
    this.g = g;
    folder.mkdirs(); //make the Genesis folder if it doesn't exist
    checkFiles(); //make the files
    lout = new PrintWriter(new FileWriter(log, true));
    rout = new PrintWriter(new FileWriter(resp, true));
}

private void checkFiles() { //create new log and response files if they don't exist
    try {
        if (!log.exists())
            log.createNewFile();
        if (!resp.exists())
            resp.createNewFile();
    } catch (IOException e) {
        g.printError(e);
    }
}

public Genesis getGenesis() {
    return g;
}

public File getFolder() {
    return folder;
}

public File getLog() {
    return log;
}

public File getResponsesFile() {
    return resp;
}

public HashMap<ResponseType, List<String>> getResponses() { //get a hashmap of all responsetypes and responses for that type in list form
    checkFiles(); //make sure our files exist first
    HashMap<ResponseType, List<String>> res = new HashMap<ResponseType, List<String>>();
    if (resp.length() == 0) //if we don't have anything, don't return anything
        return res;
    try (BufferedReader r = new BufferedReader(new FileReader(resp))) {
        String line;
        while ((line = r.readLine()) != null) {
            for (ResponseType rt : ResponseType.values()) {
                if (line.split("�")[0].equalsIgnoreCase(rt.name())) { //I could use a different character... but it kept changing back when I moved the project between computers
                    String response = "";
                    for (int i = 1; i < line.split("�").length; i++)
                        response = line.split("�")[i].trim();
                    if (res.get(rt) == null) {
                        List<String> list = new ArrayList<String>();
                        list.add(response);
                        res.put(rt, list);
                    } else
                        res.get(rt).add(response);
                }
            }
        }
    } catch (IOException e) {
        g.printError(e);
    }
    return res;
}

public List<String> getResponses(ResponseType rt) { //get all the responses in list form for a certain response type
    checkFiles();
    List<String> res = new ArrayList<String>();
    if (resp.length() == 0)
        return res;
    try (BufferedReader r = new BufferedReader(new FileReader(resp))) {
        String line;
        while ((line = r.readLine()) != null) {
            if (line.split("�")[0].equalsIgnoreCase(rt.name()))
                res.add(line.split("�")[1].trim());
        }
    } catch (Throwable t) {
        g.printError(t);
    }
    return res;
}

public void setResponse(ResponseType type, String response) { //set a response for a certain type
    response = response.trim();
    try (BufferedReader r = new BufferedReader(new FileReader(resp))) {
        String s;
        while ((s = r.readLine()) != null) {
            if (s.equals(type.toString() + "� " + response))
                return;
        }
        rout.println(type.toString() + "� " + response);
        rout.flush();
    } catch (Throwable t) {
        g.printError(t);
    }
}

public void log(String message) {
    log(lout, message);
}

public static void log(Object output, String message) {
    PrintWriterStream out;
    if(output instanceof PrintWriter)
        out = new PrintWriterStream((PrintWriter) output);
    else if(output instanceof PrintStream)
        out = new PrintWriterStream((PrintStream) output);
    else
        throw new IllegalArgumentException("Output must be of type PrintWriter or PrintStream");
    Calendar c = Calendar.getInstance();
    String hour = c.get(Calendar.HOUR) + "";
    String minute = c.get(Calendar.MINUTE) + "";
    String second = c.get(Calendar.SECOND) + "";
    int ampm = c.get(Calendar.AM_PM);

    if(Integer.parseInt(hour) < 10) //keep the hours minutes and seconds 2 digits
        hour = "0" + hour;
    if(Integer.parseInt(minute) < 10)
        minute = "0" + minute;
    if(Integer.parseInt(second) < 10)
        second = "0" + second;

    String timestamp = "[" + hour + ":" + minute + ":" + second + ":" + (ampm == 0 ? "AM" : "PM") + "]";

    out.println(timestamp + ": " + message);
    out.flush();
}

public PrintWriter getLogStream() {
    return lout;
}

public PrintWriter getResponseStream() {
    return rout;
}

public void close() { //close the log and response file streams
    lout.close();
    rout.close();
}

}

ResponseType.java

package genesis;

public enum ResponseType {

    GREETING, FAREWELL, VALUE, LAUGH;

    public String toString() {
        return name().toLowerCase().replaceFirst(name().substring(0, 1).toLowerCase(), name().substring(0, 1).toUpperCase());
    }

    public static ResponseType getResponseType(String name) {
        for (ResponseType r : values()) {
            if(r.name().equalsIgnoreCase(name))
                return r;
        }
        return null;
    }

}

As a side note, you can compile this code yourself and export it as a runnable JAR file. It creates a folder called Genesis wherever the JAR is, which consists of a log.txt file and a responses.txt file. Log file is what you think it is, and responses file is for use by Genesis, so it knows how to respond to you over time.

Rules for talking to it correctly are as follows:

  • First message must be a greeting (hello/hi/etc)
  • Last message must be a farewell (goodbye/bye/etc)
  • After your farewell, type "exit" to exit the program correctly
\$\endgroup\$
13
\$\begingroup\$

Genesis

  • It's best practices to let statements like if, for, try ... be followed by a space to disinguish them from method invocations.

  • protected String last = null;

    • protected is pointless since Genesis is a final class and hence can't be extended.
    • This variable holds the last what? Advancing in the code it's apparentlylastResponse. Let's rename it accordingly.
    • = null isn't necessary since uninitialized reference variables are initialized with null implicitly.
  • I'd rename s, newg, newf, newl to the clearer message, newGreeting, newFarewell, newLaugh.

  • The naming in conjunction with the inverse logic of:

        boolean notg = true;
    

    doesn't make your code clearer. I'd reverse the logic to:

        boolean isGreeting = false;
    

    The same applies to notfisFarewell.

  • Going through the class from the top raised the following questions:

    // [...] >50% h's or l's → "What are h's and l's?"

    String ek → "What is an ek?"

    String k → "What is a k?" Only 8 lines later: Ah, it's a key. Let's name it accordingly.

  • Though using the l-value of assignments is valid it doesn't make your code clearer:

        System.out.println(response = ...);
    

    Sure, it gives you the opportunity to omit the braces for the preceding if but doing that isn't recommended anyway.

  • throw new ArithmeticException()

    prints just:

        Exception in thread "main" java.lang.ArithmeticException
    

    An explaining message would be nice for the users of your program.

  • I'd rename ...Punc(), cap(), punc() to the clearer ...Punctuation(), capitalize(), punctuation().

  • The switch:

        switch((int) System.nanoTime() % 5) {
            default:
            case 0:
            case 1:
            case 2:
            case 3:
                return '.';
            case 4:
                return '!';
        }
    

    can be simplified to:

        switch((int) System.nanoTime() % 5) {
            case 4:
                return '!';
            default:
                return '.';
        }
    
  • If a read //super hacky way of ... in a comment I'm highly alerted of seeing something weird.

  • Genesis.java is rather long. I'd move all the static methods to a class GenesisUtil.

FileManager

  • If I read a class name FileManager I first think of the thing it is commonly used for: a File Manager.

  • I'd separate logging and response file handling combined in FileManager at the moment.

  • Variable names like resp, lout, rout, res aren't too descriptive. Is it responsibility? Is it loud misspelled? Is it route misspelled? Is it resolution?

    I'd call them responseFileName, logWriter, responseWriter, responses.

  • I'd rename checkFiles() to createFiles() because that's what it does. Of course, only if one of them doesn't exist. But the result is the same in any case: The files exist after invocation of this method.

  • You can shorten:

        HashMap<ResponseType, List<String>> res = new HashMap<ResponseType, List<String>>();
    

    to:

        HashMap<ResponseType, List<String>> res = new HashMap<>();
    

    using the diamond operator.

  • Why are you using Unicode's 65533D as split character?

  • It's not recommended to omit braces in one-line blocks like:

        for ( ... )
            response = ...;
    
  • I'd use:

        String.format("[%s:%s:%s:%s]", hour, minute, second, ampm == 0 ? "AM" : "PM")
    

    instead of string concatenation:

        "[" + hour + ":" + minute + ":" + second + ":" + (ampm == 0 ? "AM" : "PM") + "]"
    

    Such you also probably would have recognized easier that there is a ':' too much. And you could omit the add-zero-if-less-than-10 handling completely since the format string of String.format() can do this for you.

  • I'd make variables with just two expected values like ampm a boolean and call them isAM. If it is false it is the other.

ResponseType

  • Why do you want to get an enum constant by a string name derived from its field name? Such you're giving away the type safety enumerations give you by design.

  • I'd implement the enum with a constructor:

        public enum ResponseType {
    
            GREETING("Greetings"),
            FAREWELL("Farewell"),
            VALUE("Value"),
            LAUGH("Laugh");
    
            String string;
    
            ResponseType(String string) {
                this.string = string;
            }
    
            @Override
            public String toString() {
                return string;
            }
        }
    

Running Genesis

  • for the first time from within Eclipse Luna:

    [02:10:53:PM]: Initializing Genesis v1.0...
    [02:10:53:PM]: Generating files...
    [02:10:53:PM]: Genesis v1.0 started on Windows Vista
    You: hi
    
    [02:10:57:PM]: Genesis v1.0 shut down with error code: 1
    A fatal error occurred: java.lang.ArithmeticException: / by zero
    ...
    This doesn't look like a problem relating to Genesis. Check the above general stack trace.
    ...
    

    I have my doubts about the second to last sentence. It is related to Genesis. Always! Since it happened when running it. Your end user doesn't care about why it failed and what exactly failed internally. If he or she takes this statement seriously the next questions are: "What is it related to then? My inability to enter proper words? My programs' installations? My OS installation? My OS settings? My virus scanner? My machine? My internet?"

  • Hiding full Java stack traces from the end user is OK. Not writing them to the log file will make your support people hate you fervently.

\$\endgroup\$
  • \$\begingroup\$ Sorry for the late response, I paused my progress on the project for a week or so. This is an amazing response! I will definitely get to work on changing all of this and fixing more weird bugs and issues with the project. Thanks :) \$\endgroup\$ – DonkeyCore Aug 7 '15 at 1:10
11
\$\begingroup\$

Just a quick remark: the amount of horizontal scrolling doesn't make for very readable/maintainable code:

private static String transform(String s) { //transform a string into something the program can read
return s.toLowerCase().replace("?", "").replace(".", "").replace("!", "").replace(",", "").replace("_", "").replace("~", "").replace("`", "").replace("'", "").replace("\"", "").replace("\"", "").replace("\\", "").replace(":", "").replace(";", "").replace("the", " ").replace("teh", " ").replace("how do", "how can").replace("re", "").replace(" a ", " ").replace("is", "").replace("has", "").replace("get to", "go to").replaceAll("\\Bs\\b", "").replaceAll(" {2}?", "").trim();
}

Consider:

/*transform a string into something the program can read*/
private static String transform(String s) {
    return s.toLowerCase()
            .replace("?", "")
            .replace(".", "")
            .replace("!", "")
            .replace(",", "")
            .replace("_", "")
            .replace("~", "")
            .replace("`", "")
            .replace("'", "")
            .replace("\"", "")
            .replace("\"", "")
            .replace("\\", "")
            .replace(":", "")
            .replace(";", "")
            .replace("the", " ")
            .replace("teh", " ")
            .replace("how do", "how can")
            .replace("re", "")
            .replace(" a ", " ")
            .replace("is", "")
            .replace("has", "")
            .replace("get to", "go to")
            .replaceAll("\\Bs\\b", "")
            .replaceAll(" {2}?", "")
            .trim();
}

Notice .replace("\"", "") is called twice? If that's intentional (probably is, given they're consecutive), it deserves a comment.

I can't help thinking most of it would be more efficient and less redundant with a regex replace, though.

\$\endgroup\$
  • 1
    \$\begingroup\$ You're right ;) In Java there's String.replaceAll(String, String) that will happily perform regex-based replacement for all matches. \$\endgroup\$ – h.j.k. Jul 24 '15 at 3:38
  • \$\begingroup\$ Thanks for pointing that out, it was unintentional. I'll definitely try stacking my code vertically and messing around a bit more with regex. \$\endgroup\$ – DonkeyCore Jul 24 '15 at 3:46
1
\$\begingroup\$

There's a lot of stuff to cover here that others have already covered. Here's a few things that stand out to me:

Reuse Pattern instances

You have a few Pattern.compile calls. You should assign those to some static field and reuse them. Compiling a pattern is more expensive than using the pattern on some string. So compile once and use many times. Patterns are immutable an thread safe once compiled.

Circular dependency

You have a circular dependency between the FileManager and Genesis. Both seem to need each other. That's a huge design smell (poor cohesion, tight coupling). Break the cycle by refactoring into more cohesive classes.

Constructors

Your constructors are doing work and creating more objects and having side effects (directory creation). A good practice is to make your classes more testable by not letting them create instances for their own dependencies but instead injecting those via the constructor. You don't have to use a dependency injection framework for this. Simply create all your objects in your main method and provide them to each other via their constructors. Constructors should not do anything else than assigning things to fields.

Logging

You seem to have implemented your own logging. Probably better to use either java.util.logging or something like slf4j. This is a wheel that does not reinventing.

Output

You should get rid of the PrintWriter thing, which you rightfully commented is hacky. I'd suggest separating logic and output a bit more by adding abstractions for your input and response and then rendering those in an appropriate way. Essentially this is an event driven system where input events get handled and trigger some output and side effects. Assuming you actually want to do something more interesting than printing things on a console, this will be helpful when adapting e.g. to various chat bot APIs from slack, microsoft, and others. Also it will help making this more testable.

\$\endgroup\$
  • \$\begingroup\$ "does not reinventing" \$\endgroup\$ – CalculatorFeline Jun 19 '17 at 4:26
0
\$\begingroup\$

Replace the transform Method by something like this:

public String transform(String str)
    {
        //make sure string is not null
        if(str == null){
            return null;
        }

        //make string readable for system
        char[] replacements = ("!?;:.,\"'*@€|~{}[]()\n").toCharArray();
        for(char c: replacements){
            String rep = String.valueOf(c);
            if(str.toLowerCase().contains(rep.toLowerCase())){
                try{
                    str = str.replaceAll(rep, "");
                }catch(Exception e){}
            }
        }

        return str.trim();
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Can you expand on why this is an improvement, so that the OP can learn from your rationale/thought process? \$\endgroup\$ – forsvarir Mar 1 '17 at 11:43
  • \$\begingroup\$ Cause it's shorter and more efficient. \$\endgroup\$ – user6765242 Mar 1 '17 at 11:47

protected by Jamal Jun 18 '17 at 17:02

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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