You're using a bad practice, and by using it, you're directly affected. A method returning a List (or any kind of collection), should never return null. It should return an empty list if there's nothing to return. By returning null, you force every caller, including yourself, to always check for null before using the returned list, which you failed to do:
List<BooksEntity> books = this.getAllBooks();
if (books.isEmpty()) {
return null;
}
In the above code, you don't check if books
is null before calling isEmpty()
. And since getAllBooks()
returns null instead of an empty list in case no book is found, you get a NullPointerException.
Here's how I would rewrite your code:
@Override
public List<BooksEntity> getAllBooks() {
return em.createNamedQuery("BooksEntity.findAll").getResultList();
}
@Override
public List<BooksEntity> getAllBooksUser(String name) {
List<BooksEntity> books = this.getAllBooks();
List<BooksEntity> userbooks = new ArrayList<BooksEntity>();
for (BooksEntity book : books) {
users = book.getUsers();
for (UsersEntity user : users) {
if (name.equals(user.getName())) {
userbooks.add(book);
}
}
}
return userBooks;
}
Note how the code is much shorter, and how it doesn't have a risk of throwing a NullPointerException.
That said, your method of finding the books for a given user name is extremely inefficient: you're loading every book (imagine doing that with a real library), and for every book, you're loading all its users.
It would be much more efficient to find all the users with the given name (using the findByName
named query, for example), and return their book. Or even better, to do all this in a single JPQL query:
select book from UsersEntity user
inner join user.book book
where user.name = :name
The method would then look like this:
public List<BooksEntity> getAllBooksUser(String name) {
String jpql =
"select book from UsersEntity user"
+ " inner join user.book book"
+ " where user.name = :name";
return em.createTypedQuery(jpql, BooksEntity.class)
.setParameter("name", name)
.getResultList();
}
Finally, your code would be more readable if you named your entities Book
and User
, instead of BooksEntity
and UsersEntity
. Using the plural form for a single user or book is a really bad idea. And the Entity
suffix is annoying noise.