3
\$\begingroup\$

I made this program as a baby step to creating an animal shelter management system that will be used in a real life animal shelter. The goal is to move from text files to an online database, and from console interface to a GUI. Two things I'm yet to learn.

I set out with the goal of creating a CRUD program that stores the information in files.

The program runs with no compilation errors, and as far as I can see, no logical errors.

I am looking for any type of input. Whether it's naming, logic, or OOP choices.

Main.java

public class Main {

    public static void main(String[] args) {
        UserTextInterface userTextInterface = new UserTextInterface();
        userTextInterface.run();
    }
}

Animal.java

import java.util.Date;

public class Animal {
    private String name;
    private int ID;
    private String notes;
    private String species;
    private Date dateOfBirth;



    public Animal(String name, int ID, String notes, String species, Date dateOfBirth){
        this.name = name;
        this.ID = ID;
        this.notes = notes;
        this.species = species;
        this.dateOfBirth = dateOfBirth;
    }


    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public int getID() {
        return ID;
    }

    public void setID(int ID) {
        this.ID = ID;
    }

    public String getNotes() {
        return notes;
    }

    public void setNotes(String notes) {
        this.notes = notes;
    }

    public String getSpecies() {
        return species;
    }

    public void setSpecies(String species) {
        this.species = species;
    }

    public Date getDateOfBirth() {
        return dateOfBirth;
    }

    public void setDateOfBirth(Date dateOfBirth) {
        this.dateOfBirth = dateOfBirth;
    }

    @Override
    public String toString(){
        return "Animal: " + species + ", Name: " + name + ", ID: " + ID + ", Notes: " + notes + ", Date of birth: " + dateOfBirth.toString() ;
    }
}

UserTextInterface.java

import java.util.Scanner;

public class UserTextInterface {
    private Scanner reader;
    private InputValidator inputValidator;
    private AnimalHandler animalHandler;

    public UserTextInterface(){
        reader = new Scanner(System.in);
        inputValidator = new InputValidator();
        animalHandler = new AnimalHandler("src/animal.txt");
    }

    public void run(){
        printOptions();
        while(processInput());
        animalHandler.saveArrayToFile();
    }

    public void printOptions(){
        System.out.println("Options:");
        System.out.println("0. Quit");
        System.out.println("1. Add a new dog");
        System.out.println("2. Add a new cat");
        System.out.println("3. Print all animals");
        System.out.println("4. Delete an animal");
        System.out.println("5. Update an animal");
    }

    public boolean processInput(){
        int input = inputValidator.readInt();
        while(input != 0){
            if(input == 1){
                animalHandler.addDog();
            }
            else if(input == 2){
                animalHandler.addCat();
            }
            else if(input == 3){
                animalHandler.printAllAnimals();
            }
            else if(input == 4){
                animalHandler.removeAnimal();
            }
            else if(input == 5){
                animalHandler.updateAnimal();
            }
            else{

            }
            input = inputValidator.readInt();
        }
        return false;
    }
}

FileHandler.java

import java.io.File;
import java.io.FileWriter;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.Scanner;

public class FileHandler {
    private String filePath;
    private File filePointer;
    private Scanner reader;
    private SimpleDateFormat dateFormatter;



    public FileHandler(String filePath){
        this.filePath = filePath;
        dateFormatter = new SimpleDateFormat("dd/MM/yyyy");

    }


    public List<Animal> load(){
        List<Animal> contents = new ArrayList<Animal>();try{
            filePointer = new File(filePath);
            reader = new Scanner(filePointer);
        }
        catch(Exception e){
            return null;
        }
        while(reader.hasNextLine()){
            String line = reader.nextLine();
            String[] parts = line.split(":");
            Date dateOfBirth = new Date();
            try {
                 dateOfBirth = dateFormatter.parse(parts[4]);
            }
            catch(Exception e){
                System.out.println(e.getMessage());
            }
            contents.add(new Animal(parts[0], Integer.parseInt(parts[1]), parts[2], parts[3], dateOfBirth));
        }
        reader.close();
        return contents;
    }

    public boolean save(List<Animal> content){
        FileWriter writer;
        try{
            writer = new FileWriter(filePath);
        }
        catch(Exception e){
            return false;
        }
        for(Animal animal : content){
            try{
                writer.write(animal.getName() + ":" + animal.getID() + ":" + animal.getNotes() + ":" + animal.getSpecies() + ":" + dateFormatter.format(animal.getDateOfBirth())+ "\n");
            }
            catch (Exception e){
                return false;
            }
        }
        try{
            writer.close();
        }
        catch(Exception e){
            System.out.println(e.getMessage());
        }
        return true;
    }
}

AnimalHandler.Java

import java.text.SimpleDateFormat;
import java.util.*;

public class AnimalHandler {
    private FileHandler fileHandler;
    private List<Animal> fileArray;
    private Scanner reader;
    private InputValidator inputValidator;
    private SimpleDateFormat dateFormatter;


    public AnimalHandler(String filePath){
        fileHandler = new FileHandler(filePath);
        fileArray = new ArrayList<Animal>();;
        reader = new Scanner(System.in);
        inputValidator = new InputValidator();
        dateFormatter = new SimpleDateFormat("dd/MM/yyyy");
        loadFileToArray();
    }

    public void loadFileToArray(){
        fileArray = fileHandler.load();
    }

    public void saveArrayToFile(){
        fileHandler.save(fileArray);
    }

    public void addCat(){
        System.out.println("Enter the cat's name");
        String name = reader.nextLine();
        System.out.println("Enter the cat's ID");
        int ID = readID();
        System.out.println("Enter any notes:");
        String notes = reader.nextLine();
        Date date = readDate();
        fileArray.add(new Animal(name, ID, notes, "Cat", date));
        System.out.println("Cat added successfully");
    }



    public void addDog(){
        System.out.println("Enter the dog's name");
        String name = reader.nextLine();
        System.out.println("Enter the dog's ID");
        int ID = readID();
        System.out.println("Enter any notes:");
        String notes = reader.nextLine();
        Date date = readDate();
        fileArray.add(new Animal(name, ID, notes, "Dog", date));
        System.out.println("Dog added successfully");
    }

    public void removeAnimal(){
        System.out.println("Enter the ID of the animal you want to remove: ");
        int ID = inputValidator.readInt();
        for(Animal animal : fileArray){
            if(animal.getID() == ID){
                fileArray.remove(animal);
                System.out.println("Animal: "+ animal.getSpecies() +  ", Name: " + animal.getName() + ": Deleted successfully");
                return;
            }
        }
        System.out.println("Could not find animal with ID: " + ID);
    }


    public void updateAnimal(){
        System.out.println("Enter the ID of the animal you want to update: ");
        int ID = inputValidator.readInt();
        if(!IDAlreadyExists(ID)){
            System.out.println("Could not find animal with ID: " + ID);
            return;
        }

        int index = -1;
        for(Animal animal : fileArray){
            if(animal.getID() == ID){
                index = fileArray.indexOf(animal);
                break;
            }
        }

        if(index >= 0){
            Animal animal = fileArray.get(index);
            System.out.println("Found: Animal: " + animal.getSpecies()  + ", Name: " + animal.getName() + ". Please enter new details below");

            System.out.println("Would you like to enter a new name? Y/N: ");
            String answer = reader.nextLine();
            if (answer.equalsIgnoreCase("y")) {
                System.out.println("Enter a new name: ");
                String newName = reader.nextLine();
                animal.setName(newName);

            }


            System.out.println("Would you like to enter a new ID? Y/N: ");
            answer = reader.nextLine();
            if (answer.equalsIgnoreCase("y")) {
                System.out.println("Enter a new ID: ");
                int newID = inputValidator.readInt();
                if(IDAlreadyExists(newID)){
                    System.out.println("This ID is already in use. Please enter a different one: ");
                    newID = readID();
                }
                animal.setID(newID);

            }


            System.out.println("Would you like to enter a new date of birth? Y/N: ");
            answer = reader.nextLine();
            if(answer.equalsIgnoreCase("y")){
                System.out.println("Enter a new date of birth: ");
                Date newDate = readDate();
                animal.setDateOfBirth(newDate);
            }
        }
        else{
            System.out.println("Error, ID not found. This should never happen.");
        }
    }

    public void printAllAnimals(){
        for(Animal animal : fileArray){
            System.out.println(animal);
        }
    }

    public boolean IDAlreadyExists(int ID){
        for(Animal animal: fileArray){
            if(animal.getID() == ID){
                return true;
            }
        }
        return false;
    }

    public int readID(){
        int ID = inputValidator.readInt();
        while(IDAlreadyExists(ID) || ID < 0){
            if(ID < 0){
                System.out.println("The ID cannot be negative. Please enter a positive ID: ");
            }
            if(IDAlreadyExists(ID)){
                System.out.println("This ID is already in use. Please enter a different one: ");
            }
            ID = inputValidator.readInt();
        }
        return ID;
    }

    public Date readDate() {

        String date = "";

        System.out.println("Enter day of birth:");
        int day = inputValidator.readInt();
        while (day < 1 || day > 31) {
            System.out.println("Invalid number. Please enter a number between 1-31");
            day = inputValidator.readInt();
        }
        if (day < 10) {
            date += "0" + day + "/";
        } else {
            date += day + "/";
        }

        System.out.println("Enter month of birth:");
        int month = inputValidator.readInt();
        while (month < 1 || month > 12) {
            System.out.println("Invalid number. Please enter a number between 1-12");
            month = inputValidator.readInt();
        }
        if (month < 10) {
            date += "0" + month + "/";
        } else {
            date += month + "/";
        }


        System.out.println("Enter year of birth:");
        int year = inputValidator.readInt();
        int currentYear = Calendar.getInstance().get(Calendar.YEAR);
        while(year < 1900 || year > currentYear){
            System.out.println("Invalid number. Please enter a number between 1900 and " + currentYear);
            year = inputValidator.readInt();
        }
        date += year + "";

        Date dateOfBirth = new Date();
        try {
            dateOfBirth = dateFormatter.parse(date);
        }
        catch(Exception e){
            System.out.println(e.getMessage());
        }

        return dateOfBirth;
    }
}

InputValidator.java

import java.util.Scanner;

public class InputValidator {
    private Scanner reader;

    public InputValidator(){
        reader = new Scanner(System.in);
    }

    public int readInt(){
        int input = 0;
        boolean gotCorrect = false;
        while(!gotCorrect) {
            try {
                input = Integer.parseInt(reader.nextLine());
                gotCorrect = true;
            } catch (Exception e) {
                System.out.println("Please enter a valid number");
            }
        }
        return input;
    }
}

The text file where the information is stored has the source src/animal.txt

\$\endgroup\$
  • \$\begingroup\$ It's my personal opinion that this reads an awful lot like one of Oracle's Java certification questions. Can anyone with more recent experience suggest that the certification questions have moved in a different direction? \$\endgroup\$ – Eric Stein Jul 13 at 12:55
3
\$\begingroup\$

Unfortunately I don't have time to finish my answer right now, but here's what I think so far:

Main.java

If your whole program is built around reading and manipulating a single file, it's customary and useful to take the name of that file in as a parameter (i.e. read it from args) instead of hardcoding it. (In future versions you might replace the filename with, say, the connection information for the database.)

Animal.java

No real objections to the current code. This would be a good place to put a Species enum. (See AnimalHandler.java).

UserTextInterface.java

  • The only reason processInput() returns a boolean is to break out of the one-line while-loop in run(), but the while-loop in run() literally does nothing. It doesn't even call processInput() more than once, since processInput() will never return true. So just give processInput() a void return type and have run() call it normally. It's exactly the same behavior.
  • In processInput(), the if-statement in the while-loop ends with an empty branch (else { }). This is unnecessary and should be removed.

AnimalHandler.java

  • A bit off-topic: this code loads all the data from your "database" (the file) into an in-program, in-memory structure (fileArray). This is fine for the current program, but since you say you want to use an online database in the future, I feel it's only fair to warn you that this pattern would not work well for that case. It would produce a very slow program. Ideally you would want some kind of database-interface class with useful methods like findAnimal(int ID), updateAnimal(Animal animal), etc., that could be then be used by an interact-with-users class.
  • fileArray is a bad name for your List object. Arrays and lists are not the same thing in Java, and fileArray sounds more like an array of files than an array of things from a file.
  • You spend a lot of time searching for animals by ID by looping through fileArray and checking each ID individually. This will become increasingly inefficient as the list of animals grows longer. It would be far more efficient to replace the List with a Map, using the IDs as the keys and the Animals as the values.
  • Since the addDog() and addCat() methods are doing pretty much the same thing, you could combine them a more generic addAnimal() method that takes a species parameter. I think that a string would be good enough at present (you can capitalize or de-capitalize the first letter as needed), but you could also create a new Species enum with useful properties. This will make the code easier to extend if you decides to add more animal types in the future (hamsters, goldfish, parrots, etc.).
    • Frankly, even the enum is not ideal, since you have to recompile the code every time you want to add a new species. It would be even better if you had a proper Species class and were able to load the species data from an external source as well (file, database, administrator input, etc.).
  • In readDate(), instead of building the date string as you go, you can wait until the end and use String.format() to build the string from all your integers at once. With the right format string, this will also do all the zero-padding that you're currently doing with if-statements. String date = String.format("%02d/%02d/%d", day, month, year);
    • Actually, I'm not 100% sure that dateFormatter.parse() even needs the zero-padding. Need to do a little more research on that.
  • A bit off-topic again: I see that you have two separate but basically-identical SimpleDateFormat objects based on the pattern "dd/MM/yyyy", one in this file and one in FileHandler. I just want to emphasize that this is a good thing. AnimalHandler does not know how FileHandler handles reading dates from or writing dates to the file, and it doesn't need to know that. You could change the pattern for one SimpleDateFormat and it wouldn't matter at all to the other one. Encapsulation is good.
\$\endgroup\$
2
\$\begingroup\$

OOP considerations:

Dog and Cat are both animals; You should have Dog and Cat classes which extend Animal. You have basically three concerns (business logic) in your application:

(1) Get input from command line,

(2) Validate inputs and translate inputs to entity,

(3) Apply CRUD operations to perform on entities. You can create one interface for each of these.

interface CommandHandler {
    handleCommand(AddAnimalCommand command);
    handleCommand(UpdateAnimalCommand command);
    //...
}

Validator (this keeps all your validation logic in one place):

//Return error message if not valid else return EMPTY_STRING (Don't ever return null!)
interface Validator {
   String validateId(int id);
   String validateDate(String date);
}

DataStore (this provides flexibility of moving from file storage to other storage systems e.g. DB, Excel etc), you just create one more implementation class and use that:

interface DataStore {
  void save(Animal animal);
  Animal get(int id);
  void update(Animal animal);
  void delete(int id);
}

There are few observations in your coding:

readID() could be simplified.

 int readId(){
    while(true){
        int id = readInt();
        String err = validator.validate(id);
        if(validator.isEmpty()) return id;
        System.out.println(err);
    }
 }

Could you not take dateOfBirth in a specific format from user? Like 'Enter date of birth in year-month-day (for example 1990-03-10) format.'?

Remember to break the code into interfaces with clear functionality (one responsibility), implementing classes, helper classes, and methods which are not long and have meaningful names. AnimalHandler does not look a good name. I will advise to install SONAR in your IDE (better if you are using IntelliJ), it highlights bad coding practices, can sometimes report possibility of NullPointerException too.

\$\endgroup\$
  • \$\begingroup\$ What I did at first is have Dog and Cat extend Animal. But it made things more complicated, and I was advised against it on the /r/learnprogramming Discord server. The reasoning was that there are many more animals than dogs and cats, and it can spiral out of control. \$\endgroup\$ – Ammir Barakat Jul 13 at 17:14
  • \$\begingroup\$ Having classes like Dog and Cat gives you an advantage of flexibility, you can handle different animals differently. However, in your current requirement scope this is not clear, so you can instead have enum AnimalType { Dog, Cat } or Species seems fine too, just make it enum. \$\endgroup\$ – Satyendra Kumar Jul 13 at 17:23
2
\$\begingroup\$

Animal.java

  • I do not know which version of Java you are using, but consider version 8 as it is pretty much the industry standard now and use LocalDate instead of Date
  • species seems like it should be an Enum, not a String

InputValidator.java

  • Keep your scopes to smallest possible, why is Scanner an instance field?
  • Do not forget to close your Scanner
public class InputValidator {
    public int readInt() {
        Scanner sc = new Scanner(System.in);
        int input = 0;
        boolean gotCorrect = false;
        while (!gotCorrect) {
            try {
                input = Integer.parseInt(sc.nextLine());
                gotCorrect = true;
            } catch (Exception e) {
                System.out.println("Please enter a valid number");
            }
        }
        sc.close();
        return input;
    }
}
  • With all this said, I am not super convinced this should be a class, lets get back to this later

FileHandler.java

  • I will omit save and make comments on load
  • Again we are using LocalDate and keeping our scopes small
  • Inform the user when an Animal can not be loaded, do not add it with just wrong date of birth
  • Do not forget to close your Closables
public class FileHandler {

    private DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("dd/MM/yyyy");

    public List<Animal> load(String filePath) {
        try (Scanner scanner = new Scanner(new File(filePath))) {
            List<Animal> contents = new ArrayList<>();
            while (scanner.hasNextLine()) {
                String[] parts = scanner.nextLine().split(":");
                try {
                    LocalDate dateOfBirth = LocalDate.parse((parts[4]), dateFormatter);
                    contents.add(new Animal(parts[0], Integer.parseInt(parts[1]), parts[2], parts[3], dateOfBirth));
                } catch (Exception e) {
                    System.out.println("Animal: " + parts[0] + " could not be read!");
                    System.out.println(e.getMessage());
                }
            }
            return contents;
        } catch (FileNotFoundException e) {
            e.printStackTrace();
            return new ArrayList<>();
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

I think this bit is worth refactoring:

    public void printOptions(){
        System.out.println("Options:");
        System.out.println("0. Quit");
        System.out.println("1. Add a new dog");
        System.out.println("2. Add a new cat");
        System.out.println("3. Print all animals");
        System.out.println("4. Delete an animal");
        System.out.println("5. Update an animal");
    }

    public boolean processInput(){
        int input = inputValidator.readInt();
        while(input != 0){
            if(input == 1){
                animalHandler.addDog();
            }
            else if(input == 2){
                animalHandler.addCat();
            }
            else if(input == 3){
                animalHandler.printAllAnimals();
            }
            else if(input == 4){
                animalHandler.removeAnimal();
            }
            else if(input == 5){
                animalHandler.updateAnimal();
            }
            else{

            }
            input = inputValidator.readInt();
        }
        return false;
    }
}

Suppose your client comes to you and tells you they want to be able to do 10 more things with the data from the database. What you have now suffices for a small set of operations but is not scalable. Each new operation will require a print statement for the prompt and an addition to the processInput, making the function monstrous and difficult to manage.

Though I don't know how the design would go, I would suggest creating an Option class hierarchy of some sort, where an Option has a prompt, a handler (maybe a class instance that it owns? Not sure), and a mechanism by which outside classes can "select" that option.

I'm sure there's a better way to do this, though, but that's the best alternative I can think of at the moment.

Edit: another issue with your approach is the reliance on magic numbers for the menu selections that the user inputs (e.g., the logic that 1 == "Add new dog"). There's a disconnect between the strings used to represent the menu options and the numbers you use to denote those options. If you rearrange the menu options, you'll have to very carefully review the code to ensure that nothing breaks. I'd recommend defining these using an enum instead.

\$\endgroup\$

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.