Who is this Chatbot?

Question

Based off the coding puzzle: Who is this chatbot?

The code's purpose is to create an AI that the user can communicate with. Due to there being no hard-coded strings, the AI learns from what the user has said.

In my version of this AI, all messages said by the user are stored in an array and when it comes time for the computer to speak, a random message from the array is written to standard output.

The code

Main.java

public class Main {
    public static void main(String[] args) {
        String name;
        if(args.length >= 1) {
            name = args[0];
        } else {
            name = "You";
        }
        Conversation c = new Conversation(new AI(), new User(name));
        while(true) {
            String input = c.getUser();
            if(input.equals(":exit")) {
                System.exit(0);
            }
            c.getAI();
        }
    }
}

Converstaion.java

public class Conversation {
    private AI ai;
    private User user;

    private DynamicArray saidStrings = new DynamicArray(1);
    private int saidCount = 0;

    public Conversation(AI a, User u) {
        ai = a;
        user = u;
    }

    public String getUser() {
        String input = user.getInput();
        saidStrings.push(input);
        saidCount++;
        return input;
    }
    public void getAI() {
        ai.saySomething(saidStrings, saidCount);
    }
}

AI.java

import java.util.Random;

public class AI {
    private Random rand = new Random();

    public void saySomething(DynamicArray saidStrings, int saidCount) {
        System.out.printf("Bot: %s\n", saidStrings.get(getRandom(saidCount)));
    }
    public int getRandom(int max) {
        return rand.nextInt(max);
    }
}

User.java

import java.util.Scanner;

public class User {
    private Scanner input = new Scanner(System.in);

    private String name;

    public User(String n) {
        name = n;
    }

    public String getInput() {
        System.out.printf("%s: ", name);
        return input.nextLine();
    }
}

DynamicArray.java

/*

Strongly based off of: http://stackoverflow.com/questions/3536153/c-dynamically-growing-array

*/

public class DynamicArray {
    private String[] array;
    private int used;
    private int size;

    public DynamicArray(int initialSize) {
        array = new String[initialSize];
        used = 0;
        size = initialSize;
    }

    public void push(String e) {
        if(used == size) { // if the limit has been reached
            size *= 2;

            String[] temp = array; //  a median for the trade
            array = new String[size];
            copy(temp, array);
        }
        array[used++] = e; // append the provided element
    }

    public String get(int i) {
        return array[i];
    }

    private void copy(String[] src, String[] dest) {
        System.arraycopy(src, 0, dest, 0, src.length);
    }
}

Concerns:

• Am I following common OOP practices?

• Am I following common Java practices

• Do I have too many classes?

• Do I have too few classes?

• Am I handling input and output in the correct places?

• Do I have too few/many comments?

Any other recommendations for improvement are welcome.

Answer 1

Your DynamicArray class is mimicking an ArrayList which is exactly that: a dynamic array. Use this instead of reinventing the wheel.

---

Use break; instead of System.exit(0) to exit a loop. It leaves room to add more instructions beyond the loop without having to change the implementation. It's also a more natural way to break out of a loop.

---

getUser() and getAI() indicate a return type of User and AI respectively. Choose naming that indicates you are receiving their input.

---

Write your parameter names in full (user instead of u, name instead of n).

---

Adhere to the single responsibility principle. Printing output and receiving input are two distinct tasks and shouldn't be done in the same method.

---

Concerning OO principles: perhaps create an interface Conversationist that defines a method String getSentence(). Now your User can implement this as receiving console input while the AI implements it by taking a random value from its stored text. This makes sure you can just call getSentence() for both objects and introduces some inheritance structure.

Answer 2

In addition to what @Jeroen already said...

The User class

It's not so good that the User class initializes a Scanner from System.in. The fact that this class reads from the console is a hidden requirement that might not be obvious to users using this class.

Also, is it really important for this class to read from the console? Wouldn't it be good enough to read from a Scanner? It would, as that would be a more general logic. You could pass in a Scanner in the constructor, which will eliminate the hidden requirement, and it will also open the possibility to unit testing.

By the same logic, it would be good to make System.out a parameter too. With both System.in and System.out as parameters, the hidden requirements disappear, the class becomes testable, and even it's purpose becomes perfectly clear: its responsibility is communicating with a user, so perhaps you could rename it accordingly, for example to UserInputOutputHandler or something like that.

Since the name field never changes, it would be good to make it final.

Putting the above together, an example snippet:

class UserInputOutputHandler {
    private final String name;
    private final Scanner scanner;
    private final PrintStream output;

    public UserInputOutputHandler(String name, Scanner scanner, PrintStream output) {
        this.name = name;
        this.scanner = scanner;
        this.output = output;
    }

    public String getInput() {
        output.printf("%s: ", name);
        return scanner.nextLine();
    }
}

Then you could create instances like this:

new UserInputOutputHandler("Jack", new Scanner(System.in), System.out);

I think it's a good thing that the class will not be tied to System, which essentially represents global state, making the console input manifest as a hidden requirement for this class to work.

"Converstaion"

I suppose you meant to name this Conversation...

If you change DynamicArray to a List as Jeroen suggested, note that the saidCount variable becomes unnecessary, as you'll be able to get that from saidStrings.size().

AI

The getRandom method looks rather pointless, it seems it would be simpler to just inline the random.nextInt call at the single place where it's actually used.

Class design

It seems a bit strange that Conversation stores all the messages said by the user, and passes the complete list to the AI in every interaction. Looks like they are close buddies, and indeed too tightly close.

It would be better to reduce the role of Conversation, to treat all participants as a "Member", and if they have some funky implementation based on the messages that were said (such as the bot repeating random things), let them take care of the implementation by themselves, without the extra help of getting the message history dumped on them on every interaction.

Answer 3

Let's start by addressing your concern of not following object orientation. It's ok to have two separate classes for the user and for the AI, but why don't you give them a shared interface? Something like:

interface ConversationParticipant
{
  String GetMessage();
  void NotifyMessage(String message);
}

If you do that you're going to be able to adapt your Conversation class to have any possible combination of users and AIs.

I think I'd also move saidStrings from Conversation to AI. Why would you need it in conversation? You're not keeping an history of the past messages.

I'd also try to separate the logic of your application with input/output. Wouldn't it be better to have a class that decides what your AI thinks and another that decides where its output goes to? Also, your User class should not be coupled to the console input/output. Your users would love to eventually get a graphical user interface :)