For every entity, I am making a controller, a service and a DAO. I have got now around 8 entities with those classes. Let's take, for example, my classes Categorie
and Product
.
Class CategorieDaoImpl
implements methods from CategorieDao
@Override
public boolean insertCategorie(Categorie categorie) {
Session session = null;
try {
session = super.getConnection();
session.getTransaction().begin();
session.save(categorie);
session.getTransaction().commit();
return true;
} catch (HibernateException e) {
e.printStackTrace();
return false;
} finally {
closeConnection(session);
}
}
Class ProductDaoImpl
implements methods from ProductDao
@Override
public boolean insertProduct(Product product) {
Session session = null;
try {
session = super.getConnection();
session.getTransaction().begin();
session.save(product);
session.getTransaction().commit();
return true;
} catch (HibernateException e) {
e.printStackTrace();
return false;
}
finally {
closeConnection(session);
}
}
As you can see, the code of the two classes is pretty similar, besides the parameters and the parameters of save()
.
Intellij is telling me that this is duplicate code, but it isn't giving me a solution as it normally does when the duplicate code is inside a class. Any idea how I can solve this and make it better?
Thanks in advance.
EDIT:
Most Dao classes have the same CRUS methods: get, insert, update, delete. Mostly, only the parameter is different.
One could solve the problem with an AbstractInsertable<T>
-class that has some public boolean insert(T t)
holding your code:
public abstract class AbstractInsertable<T> extends ... {
public boolean insert(T t) {
Session session = null;
try {
session = super.getConnection();
session.getTransaction().begin();
session.save(t);
session.getTransaction().commit();
return true;
} catch (HibernateException e) {
e.printStackTrace();
return false;
} finally {
closeConnection(session);
}
}
[...]
}
The implementations could then inherit from that AbstractInsertable<T>
, e.g. CategorieDaoImpl extends AbstractInsertable<Category>
. This, of course, only works as long as one only inherit from one class.
The other alternative would be to work with interfaces and default implementations.
The answer ends here. The rest is my personal oppinion.
My personal wish would be for Java to allow multi-inheritance for those exact problems: One could define one class for each CRUD-operation and use them as mixins in the Dao
-implementations. Default methods in interfaces are pretty close to multi-inheritance, but bear some limitations, e.g. all methods have to be public
and no attributes can defined, which would not be present with multi-inheritance.
A minor remark on your code: you have a possibility for a NullPointerException
to occur:
Session session = null;
try {
session = super.getConnection();
[...]
} finally {
closeSession(session);
}
Without knowing the exact implementation of closeSession(...)
, I would not be surprise if you were not performing a nullcheck, thus a NPE
could be thrown. If Session
is an AutoCloseable
, you could use a try-with-resources
. If it is not, you could use Optional
to your advantage:
Optional<Session> optionalSession = Optional.empty();
try {
optionalSession = Optional.of(super.getConnection());
session = optionalSession.get();
[...]
} finally {
optionalSession.ifPresent(this::closeSession);
}