Tukukkk

I've implemented a console ToDo application. I would like to hear some opinions about it, what should I change etc.

About project:

It is intended to manage everyday tasks. Application contains:

• adding users to database


• adding tasks to database


• getting list of user's tasks

It's my second project in which I use mockito, lombok, junit, slf4j.
I tried to use MVC pattern. I've written unit tests for my controller, too.

Let's start from package MODEL (there is nothing interesting, but I want to add every class).
User class (model)

package model;



import lombok.*;



@RequiredArgsConstructor

@Getter

@Setter

@EqualsAndHashCode



public class User {



    private final String name;

    private final String password;

    private int ID;

}

Task class (model)

package model;



import lombok.*;



@Getter

@Setter

@AllArgsConstructor

@ToString

@NoArgsConstructor

@EqualsAndHashCode

public class Task {

    private String taskName;

}

Here is my VIEW package:

ToDoView class(view)

package view;



import controller.ToDoEngine;

import model.Task;

import org.slf4j.Logger;

import org.slf4j.LoggerFactory;



import java.sql.SQLException;

import java.util.InputMismatchException;

import java.util.List;

import java.util.Scanner;





public class ToDoView {



    private static final int LOGIN = 1;

    private static final int REGISTER = 2;

    private Logger logger;

    private ToDoEngine toDoEngine;

    private Scanner input;

    private int option;



    public ToDoView(ToDoEngine toDoEngine) {

        this.toDoEngine = toDoEngine;

        logger = LoggerFactory.getLogger(ToDoView.class);

        input = new Scanner(System.in);

    }



    public void executeMainMenu() {

        logger.info("--MAIN MENU--");

        logger.info("1. Sign in");

        logger.info("2. Sign up");

        boolean isInvalid = true;

        while (isInvalid) {

            try {

                option = input.nextInt();

                if (option == 1 || option == 2) {

                    isInvalid = false;

                    switch (option) {

                        case LOGIN:

                            executeLoginCase();

                            break;

                        case REGISTER:

                            executeRegistrationCase();

                            break;

                    }

                }

            } catch (InputMismatchException e) {

                logger.error("Try again");

                input.next();

            } catch (SQLException e) {

                e.printStackTrace();

            }

        }

    }



    private void executeLoginCase() throws SQLException {

        String username, password;

        boolean isInvalid = true;

        do {

            logger.info("Put your username and password");

            logger.info("User: ");

            username = input.next();

            logger.info("Password: ");

            password = input.next();

            if (toDoEngine.signIn(username, password)) {

                logger.info("You've logged in");

                executeUserCase();

                isInvalid = false;

            } else

                logger.info("Bad login or password, try again");

        }

        while (isInvalid);

    }



    private void executeRegistrationCase() throws SQLException {

        String username, password;

        logger.info("Put your username and password");

        logger.info("User: ");

        username = input.next();

        logger.info("Password: ");

        password = input.next();

        if (toDoEngine.createUser(username, password))

            logger.info("User created successfully, now you can sign in!");

    }



    private void executeUserCase() throws SQLException {

        do {

            logger.info("1. Add task");

            logger.info("2. Remove task");

            logger.info("3. Get all tasks");

            logger.info("4. Quit");

            option = input.nextInt();

            executeUserOptions(option);

        }

        while (option != 4);

    }



    private void executeUserOptions(int option) throws SQLException {

        switch (option) {

            case 1:

                logger.info("Input task");

                String taskName = input.next();

                toDoEngine.addTask(taskName);

                break;

            case 2:

                logger.info("Input task");

                taskName = input.next();

                toDoEngine.deleteTask(taskName);

                break;

            case 3:

                List<Task> tasks = toDoEngine.getTasks();

                for (Task task : tasks)

                    logger.info(String.valueOf(task));

                break;

        }

    }

}

Controller package

ToDoEngine class (controller)

package controller;



import model.Task;

import model.User;

import repository.TaskActions;

import repository.UserActions;



import java.sql.SQLException;

import java.util.List;



public class ToDoEngine {



    private TaskActions taskActions;

    private UserActions userActions;

    private User connectedUser;



    public ToDoEngine(UserActions userStorage, TaskActions taskStorage) {

        this.taskActions = taskStorage;

        this.userActions = userStorage;

    }



    public boolean signIn(String username, String password) throws SQLException {

        connectedUser = new User(username, password);

        if (!userActions.signIn(connectedUser)) {

            return false;

        }

        connectedUser.setID(retrieveConnectedUserID(connectedUser));

        return true;

    }



    private int retrieveConnectedUserID(User connectedUser) throws SQLException {

        return userActions.retrieveUserID(connectedUser);

    }



    public boolean createUser(String username, String password) throws SQLException {

        return userActions.createUser(new User(username, password));

    }



    public void addTask(String taskName) throws SQLException {

        taskActions.addTask(new Task(taskName), connectedUser);

    }



    public void deleteTask(String taskName) throws SQLException {

        taskActions.deleteTask(new Task(taskName), connectedUser);

    }



    public List<Task> getTasks() throws SQLException {

        return taskActions.getTasks(connectedUser);

    }

}

Repository package.
I created 2 interfaces - UserActions and TaskActions. That's beacuse I wanted to have everything organized. (I'm not gonna paste interfaces. Just classes that implement them)

TaskRepository class (repository)

package repository;



import model.Task;

import model.User;



import java.sql.*;

import java.util.ArrayList;

import java.util.List;

import java.util.TimeZone;



public class TaskRepository implements TaskActions {



    private Connection connection;



    public TaskRepository() throws SQLException {

        connection = DriverManager.getConnection("jdbc:mysql://localhost:3306/todoapp?autoReconnect=true&serverTimezone=" + TimeZone.getDefault().getID(), "root", "password");

    }



    public boolean addTask(Task task, User user) throws SQLException {

        if (task == null)

            return false;

        PreparedStatement preparedStatement = connection.prepareStatement("INSERT into tasks (task,idUser) values (?,?) ");

        preparedStatement.setString(1, task.getTaskName());

        preparedStatement.setInt(2, user.getID());

        preparedStatement.executeUpdate();



        return true;

    }



    public boolean deleteTask(Task task, User user) throws SQLException {

        if (!doesTaskExists(task))

            return false;

        PreparedStatement preparedStatement = connection.prepareStatement("DELETE from tasks WHERE idUser=? AND task=?");

        preparedStatement.setInt(1, user.getID());

        preparedStatement.setString(2, task.getTaskName());

        preparedStatement.executeUpdate();



        return true;

    }



    public List<Task> getTasks(User connectedUser) throws SQLException {

        PreparedStatement preparedStatement = connection.prepareStatement("SELECT * FROM tasks where idUser=?");

        preparedStatement.setInt(1, connectedUser.getID());

        ResultSet result = preparedStatement.executeQuery();

        List<Task> tasks = new ArrayList<Task>();

        while (result.next()) {

            Task task = new Task();

            task.setTaskName(result.getString("task"));



            tasks.add(task);

        }

        return tasks;

    }







    public boolean doesTaskExists(Task task) throws SQLException {

        PreparedStatement preparedStatement = connection.prepareStatement("SELECT task FROM tasks WHERE task=?");

        preparedStatement.setString(1, task.getTaskName());

        ResultSet result = preparedStatement.executeQuery();

        return result.next();

    }

}

UserRepository (repository)

package repository;



import model.User;



import java.sql.*;

import java.util.TimeZone;



public class UserRepository implements UserActions {



    private Connection connection;



    public UserRepository() throws SQLException {

        connection = DriverManager.getConnection("jdbc:mysql://localhost:3306/todoapp?autoReconnect=true&serverTimezone=" + TimeZone.getDefault().getID(), "root", "password");

    }



    public boolean createUser(User user) throws SQLException {

        if (doesUserWithThatUsernameExist(user))

            return false;

        PreparedStatement preparedStatement = connection.prepareStatement("INSERT into user (username,password) values (?,?)");

        preparedStatement.setString(1, user.getName());

        preparedStatement.setString(2, user.getPassword());

        preparedStatement.executeUpdate();



        return true;

    }



    public boolean signIn(User user) throws SQLException {

        PreparedStatement preparedStatement = connection.prepareStatement("SELECT COUNT(username) FROM user WHERE username=? AND password=? LIMIT 0,1");

        preparedStatement.setString(1, user.getName());

        preparedStatement.setString(2, user.getPassword());

        ResultSet result = preparedStatement.executeQuery();

        int count = 0;

        if (result.next())

            count = result.getInt(1);



        return count >= 1;

    }



    public int retrieveUserID(User user) throws SQLException {

        PreparedStatement preparedStatement = connection.prepareStatement("SELECT id FROM user WHERE username=? AND password=?");

        preparedStatement.setString(1, user.getName());

        preparedStatement.setString(2, user.getPassword());

        ResultSet result = preparedStatement.executeQuery();

        if (result.next())

            user.setID(result.getInt("id"));



        return user.getID();

    }



    private boolean doesUserWithThatUsernameExist(User user) throws SQLException {

        PreparedStatement preparedStatement = connection.prepareStatement("SELECT username FROM user WHERE username=?");

        preparedStatement.setString(1, user.getName().toUpperCase());

        ResultSet result = preparedStatement.executeQuery();



        return result.next();

    }





}

Main class:

import controller.ToDoEngine;

import repository.TaskRepository;

import repository.UserRepository;

import view.ToDoView;



import java.sql.SQLException;



public class Main {



    public static void main(String args) throws SQLException {

        UserRepository userRepository = new UserRepository();

        TaskRepository taskRepository = new TaskRepository();

        ToDoEngine toDoEngine = new ToDoEngine(userRepository,taskRepository);

        ToDoView toDoView = new ToDoView(toDoEngine);

        toDoView.executeMainMenu();

    }

}

Here are my unit tests:

package controller;



import model.Task;

import model.User;

import org.junit.Before;

import org.junit.Test;

import org.mockito.Mock;

import org.mockito.MockitoAnnotations;

import repository.TaskActions;

import repository.UserActions;



import java.sql.SQLException;

import java.util.ArrayList;

import java.util.List;



import static org.junit.Assert.assertEquals;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



public class ToDoEngineTest {



    @Mock

    TaskActions taskActionsMock;

    @Mock

    UserActions userActionsMock;

    private ToDoEngine toDoEngine;



    @Before

    public void setup() {

        MockitoAnnotations.initMocks(this);

        toDoEngine = new ToDoEngine(userActionsMock, taskActionsMock);

    }



    @Test

    public void createUser() throws SQLException {

        User user = new User("admin", "123");

        toDoEngine.createUser("admin", "123");

        verify(userActionsMock).createUser(user);

    }



    @Test

    public void getTasks() throws SQLException {

        List<Task> tasks = new ArrayList<>();

        User user = new User("admin", "123");

        Task task = new Task("wash");

        tasks.add(task);

        when(taskActionsMock.getTasks((User) any())).thenReturn(tasks);



        List<Task> actual = toDoEngine.getTasks();

        List<Task> expected = taskActionsMock.getTasks(user);



        assertEquals(expected, actual);



    }



    @Test

    public void signIn() {

        // TODO: 2018-09-06

    }



    @Test

    public void addTask() throws SQLException {

        toDoEngine.signIn("admin", "123");

        Task taskName = new Task("wash");

        User user = new User("admin", "123");

        verify(userActionsMock).signIn(user);

        toDoEngine.addTask("wash");

        verify(taskActionsMock).addTask(taskName, user);

    }



    @Test

    public void deleteTask() throws SQLException {

        toDoEngine.signIn("admin", "123");

        Task taskName = new Task("wash");

        User user = new User("admin", "123");

        verify(userActionsMock).signIn(user);

        toDoEngine.deleteTask("wash");

        verify(taskActionsMock).deleteTask(taskName, user);

    }

}

Globally, it works. I have to admit that I am a bit skeptic on your usage of
a logger. Because a logger is dedicated to log, some can be asynchronous or
use a buffer or are just disabled. Why don't you use the System.out ?

You have some tests and is a good thing. Sadly, you are only testing the
ToDoEngine that most of the time delegate to your actions and your actions
are mocks. The most complex part of your application (the one with the biggest
number of conditions and branches) is the ToDoView. But this one is not
tested and this is difficult to do because this class receive inputs and write
to the console/logger.

A solution would be to create one abstraction for the "output" and another for
the "input". But this still looks strange to me.

After looking at a simple description of the MVC pattern, I have the feeling that
your view is also your controller:

• Model: Represents problem data and operations to be performed on it.


• View : Presents information from the Model to the user via the display.


• Controller: Interprets inputs from the user and adjust the Model or View
  accordingly.

-- https://twitter.com/coreload/status/980212512137166848/photo/1

Let's try to move the Scanner into the controller. This one will still change
what is displayed to the user via the view:

class ToDoController {

  ToDoView view;

  Scanner in;



  void start() {

    do {

      view.displayMenu();

      action = in.nextInt();

      switch (action) {

        case REGISTER:

          executeRegistrationCase();

        case LOGIN:

          executeLoginCase();

          break;

        default:

          view.showError("Invalid option");

          break;

      }

    } while ( action!=QUIT );

  }



}   

Because the ToDoView is now a dumb component you can live without testing it.
The complexity is now in your controller that Interprets inputs from the user
and adjust the .. View accordingly.

Another improvement that you can do is to create controllers for each use cases.
So that your ToDoController act more as a front controller and drive the
whole application without knowing all the details. If we go deeper is this idea,
you can end up by having one controller/action for each menu entry and each of
them will change a shared model that is the state of your application.

[1] Smart and Dumb components are mainly used in frontend development but the concept is not new.

[2] Front controller is one of the J2EE patterns, "request dispatcher" is also used for the same purpose. They are dedicated to Web applications but can be reused in other kind of systems. http://www.oracle.com/technetwork/java/frontcontroller-135648.html