5
\$\begingroup\$

I'm trying to get better at java by testing myself with different small projects as opposed to reading about them in a book. I've made a simple login/register system to try get my head around inheritance and abstract classes. All my code is below. Feel free to critique the code and tell me what you would do better. My reason for doing this is I just want to see how other people would approach this sort of project and hopefully I'll learn something.

//RegLogSysDriver.java

public class RegLogSysDriver {
    public static void main(String[] args) {
        RegLogSystem system = new RegLogSystem();
        system.run();
    }
}

//RegLogSystem.java

import java.util.Scanner;
public class RegLogSystem {
    //Instance variables
    Scanner input = new Scanner(System.in);
    String regOrLogInput;
    String name, dateOfBirth, email, password;
    
    //Methods
    public void run() {
        boolean keepGoing = true;
        while(keepGoing==true) {
            System.out.println("Do you wish to (l)ogin or (r)egister");
            regOrLogInput = input.nextLine();
            
            if(regOrLogInput.contentEquals("l")) {
                System.out.println("Enter email:\n");
                email = input.nextLine();
                System.out.println("Enter password:\n");
                password = input.nextLine();
                LoginForm lForm = new LoginForm(email, password);
                lForm.executeForm();
            }else if(regOrLogInput.contentEquals("r")) {
                System.out.println("Enter name:\n");
                name = input.nextLine();
                System.out.println("Enter date of birth e.g. (03/04/2000):\n");
                dateOfBirth = input.nextLine();
                System.out.println("Enter email:\n");
                email = input.nextLine();
                System.out.println("Enter password:\n");
                password = input.nextLine();
                RegisterForm rForm = new RegisterForm(name, dateOfBirth, email, password);
                rForm.executeForm();
            }
            System.out.println("Do you wish to shutdown the system, (y)es or (n)o?");
            String inputText = input.nextLine();
            if(inputText.contentEquals("y")) {
                keepGoing = false;
            }
        }
    }
}

//LoginForm.java

public class LoginForm extends Form{
        public LoginForm(String email, String password) {
            super(email, password);
        }
    
        @Override
        public void executeForm() {
            if(getDataBase().isUserRegistered(getEmail(), getPassword())) {
                User newUser = getDataBase().getUser(getEmail(), getPassword());
                WelcomePage welPage = new WelcomePage(newUser);
                welPage.outputMessage();
                welPage.logoutMessage();
            }else {
                //put error handling here in time.
                System.out.println("User is not registered");
            }
            
            
        }
}

//RegisterForm.java

public class RegisterForm extends Form{

    private String name;
    private String dateOfBirth;
    
    public RegisterForm(String name, String dateOfBirth, String email, String password) {
        super(email, password);
        this.name = name;
        this.dateOfBirth = dateOfBirth;
    }
    
    public String getName() {
        return this.name;
    }
    
    public String getDateOfBirth() {
        return this.dateOfBirth;
    }

    @Override
    public void executeForm() {
        User newUser = new User(getName(), getDateOfBirth(), getEmail(), getPassword());
        getDataBase().addUserToDatabase(newUser);
        WelcomePage welPage = new WelcomePage(newUser);
        welPage.outputMessage();
        welPage.logoutMessage();
        
    }

    
}

//Form.java

public abstract class Form {
    private String emailEntry;
    private String passwordEntry;
    private Database dataBase;
    
    protected Form(String emailEntry, String passwordEntry) {
        this.dataBase = new Database();//both forms are going to need access to the same database
        this.emailEntry = emailEntry;
        this.passwordEntry = passwordEntry;
    }
    
    public String getEmail() {
        return this.emailEntry;
    }
    
    public String getPassword() {
        return this.passwordEntry;
    }
    
    public Database getDataBase() {
        return this.dataBase;
    }
    
    public abstract void executeForm();
}

//User.java

public class User {
    private String name;
    private String dateOfBirth;
    private String email;
    private String password;
    
    
    public User(String name, String dateOfBirth, String email, String password) {
        this.name = name;
        this.dateOfBirth = dateOfBirth;
        this.email = email;
        this.password = password;
    }
    
    
    
    public User() {
        
    }



    public String getEmail() {
        return this.email;
    }
    
    public String getPassword() {
        return this.password;
    }
    
    public String getName() {
        return this.name;
    }
    
    public String getDateOfBirth() {
        return this.dateOfBirth;
    }
}

//Database.java

import java.util.ArrayList;

public class Database {
    private static ArrayList<User> dataBase = new ArrayList<User>();
    
    static {
        dataBase.add(new User("Tom", "12/02/09", "[email protected]", "pass"));
        dataBase.add(new User("Bryan", "01/01/01", "[email protected]", "password"));
        dataBase.add(new User("Tarence", "15/22/20", "[email protected]", "passwordismypassword"));
    }
    

    
    public boolean isUserRegistered(String email, String password) {
        boolean returnVal = false;
        for(User regedUser : dataBase) {
            if(regedUser.getEmail().contentEquals(email)&&regedUser.getPassword().contentEquals(password)) {
                returnVal = true;
            }
        }
        return returnVal;
    }
    
    public User getUser(String email, String password) {
        User returnVal = new User();
        for(User regedUser : dataBase) {
            if(regedUser.getEmail().contentEquals(email)&&regedUser.getPassword().contentEquals(password)) {
                returnVal = regedUser;
            }
        }
        return returnVal;
    }
    
    public void addUserToDatabase(User newUser) {
        dataBase.add(newUser);
    }
}

//WelcomePage.java

import java.util.Scanner;

public class WelcomePage {
    private User loggedInUser;
    private Scanner input = new Scanner(System.in);
    
    public WelcomePage(User loggingInUser) {
        this.loggedInUser = loggingInUser;
    }
    
    public void outputMessage() {
        System.out.println("Hi "+loggedInUser.getName());
    }
    
    public void logoutMessage() {
        boolean keepGoing = true;
        while(keepGoing==true) {
            System.out.println("Do you wish to logout, (y)es or (n)o");
            String inputText = input.nextLine();
            if(inputText.contentEquals("y")) {
                keepGoing=false;
            }
        }
        
    }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Well done for your first question here. :) Since you want to get familiar with abstract classes and inheritance, I am assuming that you don't know any other object-oriented programming languages. If there's anything more that you want to tell us about your current state of programming knowledge, feel free to edit the question, so we as reviewers can focus better. \$\endgroup\$ Commented Feb 1, 2021 at 22:46
  • \$\begingroup\$ Thanks, you're right in thinking I don't know any other OO programming languages at least not very well. Anything about the code at all that you think I could do better would be appreciated. Doesn't matter if it's a little out of my depth as of yet. I'll learn about it. \$\endgroup\$ Commented Feb 2, 2021 at 18:14

1 Answer 1

1
\$\begingroup\$

Looks good so far! Here are a couple of things I noticed or that I would change, in no particular order.

contentEquals()

This is used for comparison with a StringBuffer or StringBuilder. It's overkill in you case, it's better to use equals() or equalsIgnoreCase().

Loops

A lot of the time you did something like this

  boolean keepGoing = true;
        while(keepGoing==true) {
            ...
            if( ... ) {
                keepGoing=false;
            }
        }

Comparing to true or false isn't needed. If the variable is named well, this is more readable than the comparison.

  boolean keepGoing = true;
        while(keepGoing) {
            ...
            if( ... ) {
                keepGoing=false;
            }
        }

You could also just loop forever and then break out of the loop.

        while(true) {
            ...
            if( ... ) {
                break;
            }
        }

Static block

I didn't know about this, thanks! However, if loading the database takes a bit longer this causes a weird delay for the first user to login/register as the static block is executed when the class is first loaded and not on program start. I wouldn't change it for now but if this becomes a problem, you can always change the block to a static method and call that on program start, somewhere before running the RegLogSystem.

Database class usage

The way this is implemented works, but making a new Database object for every form isn't really needed. Instead, you can make everything in the Database class static and just do Database.isUserRegistered(...) instead of getDatabase().isUserRegistered(...).

isUserRegistered() and getUser()

Both methods can return as soon as they find a user that has that password and email. This way we save not only a variable but also the empty User constructor (this could also have been initialized with null instead of an invalid user objectx). By doing it like this we also don't always loop over the entire Database, saving some time.

    public boolean isUserRegistered(String email, String password) {
        for (User regedUser : dataBase) {
            if (regedUser.getEmail().contentEquals(email) && regedUser.getPassword().contentEquals(password)) {
                return true
            }
        }
        return false;
    }
    public User getUser(String email, String password) {
        for (User regedUser : dataBase) {
            if (regedUser.getEmail().contentEquals(email) && regedUser.getPassword().contentEquals(password)) {
                return regedUser;
            }
        }
        return null;
    }

You might want to watch out for the null value (or the empty user) if you decide to call getUser() without calling isUserRegistered() beforehand.

The long if-condition could also be extracted into a method of the User class to improve readability like this:

    public boolean checkLoginCreds(String email, String password) {
        return this.getEmail().equals(email) && this.getPassword().equals(password);
    }

Use like so: if (regedUser.checkLoginCreds(email, password)) ...

RegLogSystem

The instance variables shouldn't be public, from what I can see these don't need to be instance variables but could just be declared at the top of the run() method as vars internal to the function.

Just as an idea, couldn't this class also extend Form? Haven't tried it, but seeing how each form represents a "state", this could work out nicely.

Another idea would be to move the inputting into the Forms so that each one only handles its own inputs and nothing more, cleaning up the RegLogSystem. Perhaps you could even add an abstract method to the Form class for that.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the really in depth answer. I had a glimpse through it there and you've touched on some really good points. Looking forward to finish reading it later. \$\endgroup\$ Commented Feb 19, 2021 at 14:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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