A vanilla Hibernate app (no Spring)

Question

I finished my web project a while ago and decided to revisit one my first Java projects – the first project that had anything to do with databases. It looked pretty messy to my more experienced eye. For example, I created a new SessionFactory at each method call. I decided to refactor it a bit. I'm still more or less a beginner so it may still have lots of room for improvement. What would you suggest?

For example, I suspect catching all Exceptions is bad practice (as opposed to catching specific exceptions). I'm just not sure it's worth the time and the effort to write all those log statements

Using wrapper classes may also be unnecessary

It uses Hibernate's classes and interfaces on purpose (as opposed to JPA's)!

@Entity
@Table(name="users")
@Getter
@Setter
@NoArgsConstructor // yes, I'm too lazy to write a no-args constructor by myself
public class User {
    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column
    private Long id;

    @Column
    private String name;

    @Column(name = "last_name")
    private String lastName;

    @Column
    private Byte age;

    public User(String name, String lastName, Byte age) {
        this.name = name;
        this.lastName = lastName;
        this.age = age;
    }

    public User (Long id, String name, String lastName, Byte age) {
        this.id = id;
        this.name = name;
        this.lastName = lastName;
        this.age = age;
    }

    @Override
    public String toString() {
        return "User{" + "id=" + id +
                ", name='" + name + '\'' +
                ", lastName='" + lastName + '\'' +
                ", age=" + age +
                '}';
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        User user = (User) o;
        return id.equals(user.id) && name.equals(user.name) && lastName.equals(user.lastName) && age.equals(user.age);
    }

    @Override
    public int hashCode() {
        return Objects.hash(id, name, lastName, age);
    }
}

public class UtilHibernate {
    private static final String SCHEMA_NAME = "users_db";
    private static final SessionFactory sessionFactory;

    static {
        Properties properties = new Properties();
        String[][] propertiesArray = {
                {"hibernate.connection.url", "jdbc:mysql://localhost:3306/users_db"},
                {"dialect", "org.hibernate.dialect.MySQLDialect"},
                {"hibernate.connection.username", "pp_user"},
                {"hibernate.connection.password", "pp_user"},
                {"hibernate.connection.driver_class", "com.mysql.cj.jdbc.Driver"},
                {"hibernate.current_session_context_class", "thread"}
        };
        for (String[] property : propertiesArray) {
            properties.setProperty(property[0], property[1]);
        }
        sessionFactory = new Configuration()
                .setProperties(properties)
                .addAnnotatedClass(User.class)
                .buildSessionFactory();
    }

    public static String getSchemaName() {
        return SCHEMA_NAME;
    }

    public static SessionFactory getSessionFactory() {
        return sessionFactory;
    }

    public static void shutdown() {
        sessionFactory.close();
    }
}

public class UserDaoHibernateImpl implements UserDao {
    private final String schemaName = UtilHibernate.getSchemaName();
    private final Session session =
            UtilHibernate.getSessionFactory().getCurrentSession();

    @Override
    public void createUsersTable() {
        try {
            session.beginTransaction();
            session.createSQLQuery("""
                    CREATE TABLE IF NOT EXISTS :sn.users (
                      id int NOT NULL AUTO_INCREMENT,
                      name varchar(15),
                      last_name varchar(25),
                      age tinyint,
                      PRIMARY KEY (id)
                    );""")
                    .setParameter("sn", schemaName)
                    .executeUpdate();
            session.getTransaction().commit();
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
        }
    }

    @Override
    public void dropUsersTable() {
        try {
        session.beginTransaction();
        session.createSQLQuery("DROP TABLE IF EXISTS :sn.users;")
                .setParameter("sn", schemaName)
                .executeUpdate();
        session.getTransaction().commit();
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
        }
    }
    @Override
    public void save(User... users) {
        try {
            session.beginTransaction();
            for (User user : users) {
                session.save(user);
            }
            int length = users.length;
            if (length == 0) {
                System.out.println("No User instances are provided, no inserting was performed");
            } else {
                System.out.printf("%d records were successfully inserted into the database", length);
            }
            session.getTransaction().commit();
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
        }
    }

    @Override
    public void removeUserById(long id) {
        try {
            session.beginTransaction();
            int rowsAffected = session.createQuery("DELETE FROM User u WHERE u.id = :id;")
                    .setParameter("id", id)
                    .executeUpdate();
            session.getTransaction().commit();
            if (rowsAffected < 1) {
                System.out.printf("No user with id %d exists in the database, " +
                        "its removal is not possible", id);
            }
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
        }
    }

    @Override
    public User getUserById(long id) {
        User user = null;
        try {
            session.beginTransaction();
            user = session.get(User.class, id);
            if (user == null) {
                System.out.printf("No user with id %d exists in the database, " +
                        "its retrieval is not possible", id);
            }
            session.getTransaction().commit();
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
        }
        return user;
    }

    @Override
    public List<User> getAllUsers() {
        List<User> userList = new ArrayList<>();
        try {
            session.beginTransaction();
            userList = session.createQuery("SELECT u FROM User u", User.class).list();
            session.getTransaction().commit();
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
        }
        return userList;
    }

    @Override
    public void clearUsersTable() {
        try {
            session.beginTransaction();
            int rowsAffected = session.createSQLQuery("TRUNCATE :sn.users")
                                       .setParameter("sn", schemaName)
                                       .executeUpdate();
            session.getTransaction().commit();
            System.out.printf("The Users table was cleared, %d rows were removed", rowsAffected);
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
        }
    }
}

public class UserServiceHibernateImpl implements UserService {
    private final UserDao userDaoHibernate = new UserDaoHibernateImpl();
    @Override
    public void createUsersTable() {
        userDaoHibernate.createUsersTable();
    }
    @Override
    public void dropUsersTable() {
        userDaoHibernate.dropUsersTable();
    }
// you get the idea

public class Main {
    public static void main(String[] args) {
        UserService userService = new UserServiceHibernateImpl();
        User user = userService.getUserById(1); // or something else
        System.out.println(user);

        UtilHibernate.shutdown();
}

Answer 1

I suspect catching all Exceptions is bad practice (as opposed to catching specific exceptions).

It depends... As long as you have the same reaction to all kinds of possible exceptions (just logging in your case) there is nothing wrong with this catch all approach.

Duplicated constructor

Your user class has two constructors doing merely the the same. You could call the one having more parameters from the other. This would allow to declare the properties final. which in turn prevents problems in multithreaded environments.

Hard coded configurations

You have all configurations hard coded. This way you have to recompile your Program if any of that configuration changes. Concider the use of the Properties class to read configurations from the hard drive.

Session handling / code duplication

You have quite some methods where you commit the changes in the database on success but no rollback occurs in the event of an exception. That may lead to unexpected consistency problems.

Also you end each and every database action with a commit, including those that only read from the database and do no DML. On one hand this looks a bit smelly to me. On the other hand it allows for reducing code duplication like this:

interface DbAction{
  <T> T executeIn(Session session) throws Exception;
}

public class DbActionWrapper {
   private final Session session;
   public DbActionWrapper (Session session) {this.session = Ssession;}
   public <T> Optional<T> execute(DbAction dbAction){
       try {
            session.beginTransaction();
            return Optional.ofNullable(dbAction.executeIn(session));
        } catch (Exception e) {
            System.err.println("An exception was thrown: " + e.getMessage());
            return Optional.empty();
            session.getTransaction().rollback();
        } finally {
            session.getTransaction().commit();
        }
   }
}

And your code in UserDaoHibernateImpl would change to:

    public static final User NOT_FOUND = new User(-1,"John", "Doe", -1);
    private final DbActionWrapper dbAction;
    public UserDaoHibernateImpl(DbActionWrapper dbAction) {
      this.dbAction = dbAction;
    }

     @Override
    public void removeUserById(long id) {
        dbAction.execute(session-> {
            int rowsAffected = session.createQuery("DELETE FROM User u WHERE u.id = :id;")
                    .setParameter("id", id)
                    .executeUpdate();
            if (rowsAffected < 1) {
                System.out.printf("No user with id %d exists in the database, " +
                        "its removal is not possible", id);
            }
            return null; // return value is ignored
        });
    }

    @Override
    public User getUserById(long id) {
        dbAction.execute(session-> {
            Optional<User> user = Optional.ofNullable(session.get(User.class, id));
            if (!user.isPresent()) {
                System.out.printf("No user with id %d exists in the database, " +
                        "its retrieval is not possible", id);
            }
// You really should not return a literal null since it aims for NullPointerExceptions on the callers side.
// better solution would be to (re) raise an exception.
            return user.orElse(NOT_FOUND); 
        });
    }
   // ...

Unnecessary indirection

Your class UserServiceHibernateImpl is pure boilerplate code. It has no logic of its own and only delegates to the same one and only dependency (at least as fahr as you showed it...).

So the question is, why not having UserDaoHibernateImpl implementing the UserService directly and removing UserServiceHibernateImpl completely?