HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Simple Self-Learning AI

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
learningsimpleself

Problem

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(""))

Solution

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.

  • I'd also use one of the various Java logging frameworks instead of re-inventing the (logging) wheel.



-
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> res = new HashMap>();


to:

HashMap> 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 doe

Code Snippets

boolean notg = true;
boolean isGreeting = false;
System.out.println(response = ...);
Exception in thread "main" java.lang.ArithmeticException
switch((int) System.nanoTime() % 5) {
        default:
        case 0:
        case 1:
        case 2:
        case 3:
            return '.';
        case 4:
            return '!';
    }

Context

StackExchange Code Review Q#97898, answer score: 13

Revisions (0)

No revisions yet.